From e815d3f9ce0a0e0371c0852d9b911fe0611d9f3b Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Wed, 20 Apr 2022 00:06:45 +0200 Subject: [PATCH] LibJS: De-duplicate ClassFieldDefinition Records This was defined twice, despite being the very same thing: - ClassElement::ClassFieldDefinition - ECMAScriptFunctionObject::InstanceField Move the former to a new header and use it everywhere. Also update the define_field() AO to take a single field instead of separate name and initializer arguments. --- Userland/Libraries/LibJS/AST.cpp | 18 +++++----- Userland/Libraries/LibJS/AST.h | 8 +---- Userland/Libraries/LibJS/Forward.h | 1 + .../LibJS/Runtime/ClassFieldDefinition.h | 22 ++++++++++++ .../Runtime/ECMAScriptFunctionObject.cpp | 7 ---- .../LibJS/Runtime/ECMAScriptFunctionObject.h | 11 ++---- Userland/Libraries/LibJS/Runtime/Object.cpp | 36 ++++++++++++++----- Userland/Libraries/LibJS/Runtime/Object.h | 2 +- Userland/Libraries/LibJS/Runtime/VM.cpp | 2 +- 9 files changed, 66 insertions(+), 41 deletions(-) create mode 100644 Userland/Libraries/LibJS/Runtime/ClassFieldDefinition.h diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index b390528e2e..854418f9e6 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -1487,13 +1487,13 @@ Completion ClassElement::execute(Interpreter&, GlobalObject&) const VERIFY_NOT_REACHED(); } -static ThrowCompletionOr class_key_to_property_name(Interpreter& interpreter, GlobalObject& global_object, Expression const& key) +static ThrowCompletionOr class_key_to_property_name(Interpreter& interpreter, GlobalObject& global_object, Expression const& key) { if (is(key)) { auto& private_identifier = static_cast(key); auto* private_environment = interpreter.vm().running_execution_context().private_environment; VERIFY(private_environment); - return ClassElement::ClassElementName { private_environment->resolve_private_identifier(private_identifier.string()) }; + return ClassElementName { private_environment->resolve_private_identifier(private_identifier.string()) }; } auto prop_key = TRY(key.execute(interpreter, global_object)).release_value(); @@ -1502,7 +1502,7 @@ static ThrowCompletionOr class_key_to_property_n prop_key = TRY(prop_key.to_primitive(global_object, Value::PreferredType::String)); auto property_key = TRY(PropertyKey::from_value(global_object, prop_key)); - return ClassElement::ClassElementName { property_key }; + return ClassElementName { property_key }; } // 15.4.5 Runtime Semantics: MethodDefinitionEvaluation, https://tc39.es/ecma262/#sec-runtime-semantics-methoddefinitionevaluation @@ -1838,11 +1838,11 @@ ThrowCompletionOr ClassExpression::class_definition_e prototype->define_direct_property(vm.names.constructor, class_constructor, Attribute::Writable | Attribute::Configurable); - using StaticElement = Variant>; + using StaticElement = Variant>; Vector static_private_methods; Vector instance_private_methods; - Vector instance_fields; + Vector instance_fields; Vector static_elements; for (auto const& element : m_elements) { @@ -1871,7 +1871,7 @@ ThrowCompletionOr ClassExpression::class_definition_e if (!added_to_existing) container.append(move(element_value.get())); - } else if (auto* class_field_definition_ptr = element_value.get_pointer()) { + } else if (auto* class_field_definition_ptr = element_value.get_pointer()) { if (element.is_static()) static_elements.append(move(*class_field_definition_ptr)); else @@ -1892,7 +1892,7 @@ ThrowCompletionOr ClassExpression::class_definition_e MUST(class_scope->initialize_binding(global_object, binding_name, class_constructor)); for (auto& field : instance_fields) - class_constructor->add_field(field.name, field.initializer.is_null() ? nullptr : field.initializer.cell()); + class_constructor->add_field(field); for (auto& private_method : instance_private_methods) class_constructor->add_private_method(private_method); @@ -1902,8 +1902,8 @@ ThrowCompletionOr ClassExpression::class_definition_e for (auto& element : static_elements) { TRY(element.visit( - [&](ClassElement::ClassFieldDefinition& field) -> ThrowCompletionOr { - return TRY(class_constructor->define_field(field.name, field.initializer.is_null() ? nullptr : field.initializer.cell())); + [&](ClassFieldDefinition& field) -> ThrowCompletionOr { + return TRY(class_constructor->define_field(field)); }, [&](Handle static_block_function) -> ThrowCompletionOr { VERIFY(!static_block_function.is_null()); diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index 87b5a4e2af..743cd5bacd 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -19,6 +19,7 @@ #include #include #include +#include #include #include #include @@ -1248,13 +1249,6 @@ public: virtual ElementKind class_element_kind() const = 0; bool is_static() const { return m_is_static; } - using ClassElementName = Variant; - - struct ClassFieldDefinition { - ClassElementName name; - Handle initializer; - }; - // We use the Completion also as a ClassStaticBlockDefinition Record. using ClassValue = Variant; virtual ThrowCompletionOr class_element_evaluation(Interpreter&, GlobalObject&, Object& home_object) const = 0; diff --git a/Userland/Libraries/LibJS/Forward.h b/Userland/Libraries/LibJS/Forward.h index c207bed610..60adffd533 100644 --- a/Userland/Libraries/LibJS/Forward.h +++ b/Userland/Libraries/LibJS/Forward.h @@ -141,6 +141,7 @@ class BoundFunction; class Cell; class CellAllocator; class ClassExpression; +struct ClassFieldDefinition; class Completion; class Console; class DeclarativeEnvironment; diff --git a/Userland/Libraries/LibJS/Runtime/ClassFieldDefinition.h b/Userland/Libraries/LibJS/Runtime/ClassFieldDefinition.h new file mode 100644 index 0000000000..f02ecbbb1e --- /dev/null +++ b/Userland/Libraries/LibJS/Runtime/ClassFieldDefinition.h @@ -0,0 +1,22 @@ +/* + * Copyright (c) 2022, Linus Groh + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +#pragma once + +#include +#include + +namespace JS { + +using ClassElementName = Variant; + +// 6.2.10 The ClassFieldDefinition Record Specification Type, https://tc39.es/ecma262/#sec-classfielddefinition-record-specification-type +struct ClassFieldDefinition { + ClassElementName name; // [[Name]] + Handle initializer; // [[Initializer]] +}; + +} diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp index 2a35a0750c..f0f9fa8ace 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp @@ -299,8 +299,6 @@ void ECMAScriptFunctionObject::visit_edges(Visitor& visitor) for (auto& field : m_fields) { if (auto* property_key_ptr = field.name.get_pointer(); property_key_ptr && property_key_ptr->is_symbol()) visitor.visit(property_key_ptr->as_symbol()); - - visitor.visit(field.initializer); } } @@ -886,9 +884,4 @@ void ECMAScriptFunctionObject::set_name(FlyString const& name) VERIFY(success); } -void ECMAScriptFunctionObject::add_field(ClassElement::ClassElementName property_key, ECMAScriptFunctionObject* initializer) -{ - m_fields.empend(property_key, initializer); -} - } diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h index 9db88df1a1..08afd478ac 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h @@ -68,13 +68,8 @@ public: String const& source_text() const { return m_source_text; } void set_source_text(String source_text) { m_source_text = move(source_text); } - struct InstanceField { - Variant name; - ECMAScriptFunctionObject* initializer { nullptr }; - }; - - Vector const& fields() const { return m_fields; } - void add_field(Variant property_key, ECMAScriptFunctionObject* initializer); + Vector const& fields() const { return m_fields; } + void add_field(ClassFieldDefinition field) { m_fields.append(move(field)); } Vector const& private_methods() const { return m_private_methods; } void add_private_method(PrivateElement method) { m_private_methods.append(move(method)); }; @@ -123,7 +118,7 @@ private: ScriptOrModule m_script_or_module; // [[ScriptOrModule]] Object* m_home_object { nullptr }; // [[HomeObject]] String m_source_text; // [[SourceText]] - Vector m_fields; // [[Fields]] + Vector m_fields; // [[Fields]] Vector m_private_methods; // [[PrivateMethods]] Variant m_class_field_initializer_name; // [[ClassFieldInitializerName]] ConstructorKind m_constructor_kind : 1 { ConstructorKind::Base }; // [[ConstructorKind]] diff --git a/Userland/Libraries/LibJS/Runtime/Object.cpp b/Userland/Libraries/LibJS/Runtime/Object.cpp index 956921cbcd..4644e5599a 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.cpp +++ b/Userland/Libraries/LibJS/Runtime/Object.cpp @@ -10,6 +10,7 @@ #include #include #include +#include #include #include #include @@ -556,17 +557,36 @@ ThrowCompletionOr Object::private_set(PrivateName const& name, Value value } // 7.3.32 DefineField ( receiver, fieldRecord ), https://tc39.es/ecma262/#sec-definefield -ThrowCompletionOr Object::define_field(Variant name, ECMAScriptFunctionObject* initializer) +ThrowCompletionOr Object::define_field(ClassFieldDefinition const& field) { - Value init_value = js_undefined(); - if (initializer) - init_value = TRY(call(global_object(), *initializer, this)); + // 1. Let fieldName be fieldRecord.[[Name]]. + auto const& field_name = field.name; - if (auto* property_key_ptr = name.get_pointer()) - TRY(create_data_property_or_throw(*property_key_ptr, init_value)); - else - TRY(private_field_add(name.get(), init_value)); + // 2. Let initializer be fieldRecord.[[Initializer]]. + auto const& initializer = field.initializer; + auto init_value = js_undefined(); + + // 3. If initializer is not empty, then + if (!initializer.is_null()) { + // a. Let initValue be ? Call(initializer, receiver). + init_value = TRY(call(global_object(), initializer.cell(), this)); + } + // 4. Else, let initValue be undefined. + + // 5. If fieldName is a Private Name, then + if (field_name.has()) { + // a. Perform ? PrivateFieldAdd(receiver, fieldName, initValue). + TRY(private_field_add(field_name.get(), init_value)); + } + // 6. Else, + else { + // a. Assert: IsPropertyKey(fieldName) is true. + // b. Perform ? CreateDataPropertyOrThrow(receiver, fieldName, initValue). + TRY(create_data_property_or_throw(field_name.get(), init_value)); + } + + // 7. Return unused. return {}; } diff --git a/Userland/Libraries/LibJS/Runtime/Object.h b/Userland/Libraries/LibJS/Runtime/Object.h index 1aa7d361ad..1827df89f2 100644 --- a/Userland/Libraries/LibJS/Runtime/Object.h +++ b/Userland/Libraries/LibJS/Runtime/Object.h @@ -110,7 +110,7 @@ public: ThrowCompletionOr private_method_or_accessor_add(PrivateElement element); ThrowCompletionOr private_get(PrivateName const& name); ThrowCompletionOr private_set(PrivateName const& name, Value value); - ThrowCompletionOr define_field(Variant name, ECMAScriptFunctionObject* initializer); + ThrowCompletionOr define_field(ClassFieldDefinition const&); // 10.1 Ordinary Object Internal Methods and Internal Slots, https://tc39.es/ecma262/#sec-ordinary-object-internal-methods-and-internal-slots diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index 31509b65e4..841c0ae0dd 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -600,7 +600,7 @@ ThrowCompletionOr VM::initialize_instance_elements(Object& object, ECMAScr TRY(object.private_method_or_accessor_add(method)); for (auto& field : constructor.fields()) - TRY(object.define_field(field.name, field.initializer)); + TRY(object.define_field(field)); return {}; }