From 317b88a8c395bf34b0290eaddeefb6afe4b584ed Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Wed, 16 Jun 2021 20:52:30 +0100 Subject: [PATCH] LibJS: Replace Object's create_empty() with create() taking a prototype This now matches the spec's OrdinaryObjectCreate() across the board: instead of implicitly setting the created object's prototype to %Object.prototype% and then in many cases setting it to a nullptr right away, it now has an 'Object* prototype' parameter with _no default value_. This makes the code easier to compare with the spec, very clear in terms of what prototype is being used as well as avoiding unnecessary shape transitions. Also fixes a couple of cases were we weren't setting the correct prototype. There's no reason to assume that the object would not be empty (as in having own properties), so let's follow our existing pattern of Type::create(...) and simply call it 'create'. --- .../Applications/Spreadsheet/JSIntegration.cpp | 6 +++--- Userland/Libraries/LibJS/AST.cpp | 5 ++--- Userland/Libraries/LibJS/Bytecode/Op.cpp | 4 ++-- .../Libraries/LibJS/Runtime/ArrayPrototype.cpp | 3 +-- .../Libraries/LibJS/Runtime/GeneratorObject.cpp | 2 +- .../LibJS/Runtime/IteratorOperations.cpp | 2 +- Userland/Libraries/LibJS/Runtime/JSONObject.cpp | 12 +++++++----- Userland/Libraries/LibJS/Runtime/Object.cpp | 14 +++++++++++--- Userland/Libraries/LibJS/Runtime/Object.h | 2 +- .../Libraries/LibJS/Runtime/ObjectConstructor.cpp | 15 ++++++++------- .../Libraries/LibJS/Runtime/ProxyConstructor.cpp | 2 +- .../Libraries/LibJS/Runtime/RegExpPrototype.cpp | 2 +- Userland/Libraries/LibJS/Runtime/VM.cpp | 5 ++--- .../LibWeb/WebAssembly/WebAssemblyObject.cpp | 3 +-- 14 files changed, 42 insertions(+), 35 deletions(-) diff --git a/Userland/Applications/Spreadsheet/JSIntegration.cpp b/Userland/Applications/Spreadsheet/JSIntegration.cpp index 8c87c6ac25e..e13cc28c2e8 100644 --- a/Userland/Applications/Spreadsheet/JSIntegration.cpp +++ b/Userland/Applications/Spreadsheet/JSIntegration.cpp @@ -263,7 +263,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::parse_cell_name) if (!position.has_value()) return JS::js_undefined(); - auto object = JS::Object::create_empty(global_object); + auto object = JS::Object::create(global_object, global_object.object_prototype()); object->put("column", JS::js_string(vm, sheet_object->m_sheet.column(position.value().column))); object->put("row", JS::Value((unsigned)position.value().row)); @@ -293,7 +293,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::current_cell_position) auto position = current_cell->position(); - auto object = JS::Object::create_empty(global_object); + auto object = JS::Object::create(global_object, global_object.object_prototype()); object->put("column", JS::js_string(vm, sheet_object->m_sheet.column(position.column))); object->put("row", JS::Value((unsigned)position.row)); @@ -377,7 +377,7 @@ JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::column_arithmetic) } WorkbookObject::WorkbookObject(Workbook& workbook) - : JS::Object(*JS::Object::create_empty(workbook.global_object())) + : JS::Object(*JS::Object::create(workbook.global_object(), workbook.global_object().object_prototype())) , m_workbook(workbook) { } diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 8a8908ed2c7..51021ef7845 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -773,7 +773,6 @@ Value ClassExpression::execute(Interpreter& interpreter, GlobalObject& global_ob return {}; } class_constructor->set_constructor_kind(Function::ConstructorKind::Derived); - Object* prototype = Object::create_empty(global_object); Object* super_constructor_prototype = nullptr; if (!super_constructor.is_null()) { @@ -787,7 +786,7 @@ Value ClassExpression::execute(Interpreter& interpreter, GlobalObject& global_ob if (super_constructor_prototype_value.is_object()) super_constructor_prototype = &super_constructor_prototype_value.as_object(); } - prototype->set_prototype(super_constructor_prototype); + auto* prototype = Object::create(global_object, super_constructor_prototype); prototype->define_property(vm.names.constructor, class_constructor, 0); if (interpreter.exception()) @@ -1683,7 +1682,7 @@ Value ObjectExpression::execute(Interpreter& interpreter, GlobalObject& global_o { InterpreterNodeScope node_scope { interpreter, *this }; - auto* object = Object::create_empty(global_object); + auto* object = Object::create(global_object, global_object.object_prototype()); for (auto& property : m_properties) { auto key = property.key().execute(interpreter, global_object); if (interpreter.exception()) diff --git a/Userland/Libraries/LibJS/Bytecode/Op.cpp b/Userland/Libraries/LibJS/Bytecode/Op.cpp index 69744b6c223..d0959adc098 100644 --- a/Userland/Libraries/LibJS/Bytecode/Op.cpp +++ b/Userland/Libraries/LibJS/Bytecode/Op.cpp @@ -130,7 +130,7 @@ void NewString::execute_impl(Bytecode::Interpreter& interpreter) const void NewObject::execute_impl(Bytecode::Interpreter& interpreter) const { - interpreter.accumulator() = Object::create_empty(interpreter.global_object()); + interpreter.accumulator() = Object::create(interpreter.global_object(), interpreter.global_object().object_prototype()); } void ConcatString::execute_impl(Bytecode::Interpreter& interpreter) const @@ -307,7 +307,7 @@ void PushLexicalEnvironment::execute_impl(Bytecode::Interpreter& interpreter) co void Yield::execute_impl(Bytecode::Interpreter& interpreter) const { auto yielded_value = interpreter.accumulator().value_or(js_undefined()); - auto object = JS::Object::create_empty(interpreter.global_object()); + auto object = JS::Object::create(interpreter.global_object(), nullptr); object->put("result", yielded_value); if (m_continuation_label.has_value()) object->put("continuation", Value(static_cast(reinterpret_cast(&m_continuation_label->block())))); diff --git a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp index d1ee2d1aa7b..f0aff26ef43 100644 --- a/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/ArrayPrototype.cpp @@ -75,8 +75,7 @@ void ArrayPrototype::initialize(GlobalObject& global_object) define_property(vm.well_known_symbol_iterator(), get(vm.names.values), attr); // 23.1.3.34 Array.prototype [ @@unscopables ], https://tc39.es/ecma262/#sec-array.prototype-@@unscopables - Object* unscopable_list = create_empty(global_object); - unscopable_list->set_prototype(nullptr); + auto* unscopable_list = Object::create(global_object, nullptr); unscopable_list->put(vm.names.copyWithin, Value(true)); unscopable_list->put(vm.names.entries, Value(true)); unscopable_list->put(vm.names.fill, Value(true)); diff --git a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp index 8fb800bf394..559d55f6c3e 100644 --- a/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/GeneratorObject.cpp @@ -86,7 +86,7 @@ Value GeneratorObject::next_impl(VM& vm, GlobalObject& global_object, Optionalput("value", previous_generated_value); if (m_done) { diff --git a/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp b/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp index 7ca56509d19..79d76212c02 100644 --- a/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp +++ b/Userland/Libraries/LibJS/Runtime/IteratorOperations.cpp @@ -124,7 +124,7 @@ MarkedValueList iterable_to_list(GlobalObject& global_object, Value iterable, Va Value create_iterator_result_object(GlobalObject& global_object, Value value, bool done) { auto& vm = global_object.vm(); - auto* object = Object::create_empty(global_object); + auto* object = Object::create(global_object, global_object.object_prototype()); object->define_property(vm.names.value, value); object->define_property(vm.names.done, Value(done)); return object; diff --git a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp index 48301766083..2405e3e46f1 100644 --- a/Userland/Libraries/LibJS/Runtime/JSONObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/JSONObject.cpp @@ -106,7 +106,7 @@ String JSONObject::stringify_impl(GlobalObject& global_object, Value value, Valu state.gap = String::empty(); } - auto* wrapper = Object::create_empty(global_object); + auto* wrapper = Object::create(global_object, global_object.object_prototype()); wrapper->define_property(String::empty(), value); if (vm.exception()) return {}; @@ -396,11 +396,12 @@ JS_DEFINE_NATIVE_FUNCTION(JSONObject::parse) } Value result = parse_json_value(global_object, json.value()); if (reviver.is_function()) { - auto* holder_object = Object::create_empty(global_object); - holder_object->define_property(String::empty(), result); + auto* root = Object::create(global_object, global_object.object_prototype()); + auto root_name = String::empty(); + root->define_property(root_name, result); if (vm.exception()) return {}; - return internalize_json_property(global_object, holder_object, String::empty(), reviver.as_function()); + return internalize_json_property(global_object, root, root_name, reviver.as_function()); } return result; } @@ -426,7 +427,7 @@ Value JSONObject::parse_json_value(GlobalObject& global_object, const JsonValue& Object* JSONObject::parse_json_object(GlobalObject& global_object, const JsonObject& json_object) { - auto* object = Object::create_empty(global_object); + auto* object = Object::create(global_object, global_object.object_prototype()); json_object.for_each_member([&](auto& key, auto& value) { object->define_property(key, parse_json_value(global_object, value)); }); @@ -443,6 +444,7 @@ Array* JSONObject::parse_json_array(GlobalObject& global_object, const JsonArray return array; } +// 25.5.1.1 InternalizeJSONProperty ( holder, name, reviver ), https://tc39.es/ecma262/#sec-internalizejsonproperty Value JSONObject::internalize_json_property(GlobalObject& global_object, Object* holder, const PropertyName& name, Function& reviver) { auto& vm = global_object.vm(); diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index d63224e665f..fee600bcba1 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -65,9 +65,15 @@ PropertyDescriptor PropertyDescriptor::from_dictionary(VM& vm, const Object& obj return descriptor; } -Object* Object::create_empty(GlobalObject& global_object) +// 10.1.12 OrdinaryObjectCreate ( proto [ , additionalInternalSlotsList ] ), https://tc39.es/ecma262/#sec-ordinaryobjectcreate +Object* Object::create(GlobalObject& global_object, Object* prototype) { - return global_object.heap().allocate(global_object, *global_object.new_object_shape()); + if (!prototype) + return global_object.heap().allocate(global_object, *global_object.empty_object_shape()); + else if (prototype == global_object.object_prototype()) + return global_object.heap().allocate(global_object, *global_object.new_object_shape()); + else + return global_object.heap().allocate(global_object, *prototype); } Object::Object(GlobalObjectTag) @@ -425,12 +431,14 @@ Value Object::get_own_property_descriptor_object(const PropertyName& property_na VERIFY(property_name.is_valid()); auto& vm = this->vm(); + auto& global_object = this->global_object(); + auto descriptor_opt = get_own_property_descriptor(property_name); if (!descriptor_opt.has_value()) return js_undefined(); auto descriptor = descriptor_opt.value(); - auto* descriptor_object = Object::create_empty(global_object()); + auto* descriptor_object = Object::create(global_object, global_object.object_prototype()); if (descriptor.is_data_descriptor()) { descriptor_object->define_property(vm.names.value, descriptor.value.value_or(js_undefined())); descriptor_object->define_property(vm.names.writable, Value(descriptor.attributes.is_writable())); diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index 444a67e84e1..64df83284db 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -40,7 +40,7 @@ struct PropertyDescriptor { class Object : public Cell { public: - static Object* create_empty(GlobalObject&); + static Object* create(GlobalObject&, Object* prototype); explicit Object(Object& prototype); explicit Object(Shape&); diff --git a/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp b/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp index 99245c513e8..d59659fce05 100644 --- a/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/ObjectConstructor.cpp @@ -62,10 +62,13 @@ ObjectConstructor::~ObjectConstructor() // 20.1.1.1 Object ( [ value ] ), https://tc39.es/ecma262/#sec-object-value Value ObjectConstructor::call() { - auto value = vm().argument(0); + auto& vm = this->vm(); + auto& global_object = this->global_object(); + + auto value = vm.argument(0); if (value.is_nullish()) - return Object::create_empty(global_object()); - return value.to_object(global_object()); + return Object::create(global_object, global_object.object_prototype()); + return value.to_object(global_object); } // 20.1.1.1 Object ( [ value ] ), https://tc39.es/ecma262/#sec-object-value @@ -191,8 +194,7 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::from_entries) if (vm.exception()) return {}; - auto* object = Object::create_empty(global_object); - object->set_prototype(global_object.object_prototype()); + auto* object = Object::create(global_object, global_object.object_prototype()); get_iterator_values(global_object, iterable, [&](Value iterator_value) { if (vm.exception()) @@ -344,8 +346,7 @@ JS_DEFINE_NATIVE_FUNCTION(ObjectConstructor::create) return {}; } - auto* object = Object::create_empty(global_object); - object->set_prototype(prototype); + auto* object = Object::create(global_object, prototype); if (!properties.is_undefined()) { object->define_properties(properties); diff --git a/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp b/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp index 2e14800e422..d948ed5bbf8 100644 --- a/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp +++ b/Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp @@ -81,7 +81,7 @@ JS_DEFINE_NATIVE_FUNCTION(ProxyConstructor::revocable) }); revoker->define_property(vm.names.length, Value(0)); - auto* result = Object::create_empty(global_object); + auto* result = Object::create(global_object, global_object.object_prototype()); result->define_property(vm.names.proxy, proxy); result->define_property(vm.names.revoke, revoker); return result; diff --git a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp index 9cb84e2164a..3bb2fab46c2 100644 --- a/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp +++ b/Userland/Libraries/LibJS/Runtime/RegExpPrototype.cpp @@ -191,7 +191,7 @@ JS_DEFINE_NATIVE_FUNCTION(RegExpPrototype::exec) Value groups = js_undefined(); if (result.n_named_capture_groups > 0) { - auto groups_object = create_empty(global_object); + auto groups_object = Object::create(global_object, nullptr); for (auto& entry : result.named_capture_group_matches[0]) groups_object->define_property(entry.key, js_string(vm, entry.value.view.to_string())); groups = move(groups_object); diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index 5e1500b04a8..3c776eaf88e 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -286,8 +286,7 @@ void VM::assign(const NonnullRefPtr& target, Value value, Global VERIFY(!property.pattern); JS::Value value_to_assign; if (property.is_rest) { - auto* rest_object = Object::create_empty(global_object); - rest_object->set_prototype(nullptr); + auto* rest_object = Object::create(global_object, nullptr); for (auto& property : object->shape().property_table()) { if (!property.value.attributes.has_enumerable()) continue; @@ -406,7 +405,7 @@ Value VM::construct(Function& function, Function& new_target, Optionalbind_this_value(global_object, new_object); if (exception()) diff --git a/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyObject.cpp b/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyObject.cpp index c071b9d705b..027c1389192 100644 --- a/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyObject.cpp +++ b/Userland/Libraries/LibWeb/WebAssembly/WebAssemblyObject.cpp @@ -362,8 +362,7 @@ void WebAssemblyInstanceObject::initialize(JS::GlobalObject& global_object) Object::initialize(global_object); VERIFY(!m_exports_object); - m_exports_object = JS::Object::create_empty(global_object); - m_exports_object->set_prototype(nullptr); + m_exports_object = JS::Object::create(global_object, nullptr); auto& instance = this->instance(); for (auto& export_ : instance.exports()) { export_.value().visit(