LibJS: Add parsing and evaluation of private fields and methods

Userland/Libraries/LibJS/AST.cpp

@@ -1091,6 +1091,9 @@ Reference MemberExpression::to_reference(Interpreter& interpreter, GlobalObject&
         property_name = PropertyName::from_value(global_object, value);
         if (interpreter.exception())
             return Reference {};
+    } else if (is<PrivateIdentifier>(*m_property)) {
+        auto& private_identifier = static_cast<PrivateIdentifier const&>(*m_property);
+        return make_private_reference(interpreter.vm(), base_value, private_identifier.string());
     } else {
         property_name = verify_cast<Identifier>(*m_property).string();
         TRY_OR_DISCARD(require_object_coercible(global_object, base_value));
@@ -1166,8 +1169,14 @@ Value ClassElement::execute(Interpreter&, GlobalObject&) const
     VERIFY_NOT_REACHED();
 }
 
-static ThrowCompletionOr<PropertyName> class_key_to_property_name(Interpreter& interpreter, GlobalObject& global_object, Expression const& key)
+static ThrowCompletionOr<ClassElement::ClassElementName> class_key_to_property_name(Interpreter& interpreter, GlobalObject& global_object, Expression const& key)
 {
+    if (is<PrivateIdentifier>(key)) {
+        auto& private_identifier = static_cast<PrivateIdentifier const&>(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()) };
+    }
 
     auto prop_key = key.execute(interpreter, global_object);
     if (auto* exception = interpreter.exception())
@@ -1179,7 +1188,7 @@ static ThrowCompletionOr<PropertyName> class_key_to_property_name(Interpreter& i
     auto property_key = PropertyName::from_value(global_object, prop_key);
     if (auto* exception = interpreter.exception())
         return throw_completion(exception->value());
-    return property_key;
+    return ClassElement::ClassElementName { property_key };
 }
 
 // 15.4.5 Runtime Semantics: MethodDefinitionEvaluation, https://tc39.es/ecma262/#sec-runtime-semantics-methoddefinitionevaluation
@@ -1194,34 +1203,61 @@ ThrowCompletionOr<ClassElement::ClassValue> ClassMethod::class_element_evaluatio
     auto& method_function = static_cast<ECMAScriptFunctionObject&>(method_value.as_function());
     method_function.set_home_object(&target);
 
-    auto set_function_name = [&](String prefix) {
-        String property_name;
-        // FIXME: Handle PrivateNames as well.
-        if (property_key.is_symbol())
-            property_name = String::formatted("[{}]", property_key.as_symbol()->description());
-        else
-            property_name = property_key.to_string();
-        update_function_name(method_value, String::formatted("{} {}", prefix, property_name));
+    auto set_function_name = [&](String prefix = "") {
+        auto property_name = property_key.visit(
+            [&](PropertyName const& property_name) -> String {
+                if (property_name.is_symbol()) {
+                    auto description = property_name.as_symbol()->description();
+                    if (description.is_empty())
+                        return "";
+                    return String::formatted("[{}]", description);
+                } else {
+                    return property_name.to_string();
+                }
+            },
+            [&](PrivateName const& private_name) -> String {
+                return private_name.description;
+            });
+
+        update_function_name(method_value, String::formatted("{}{}{}", prefix, prefix.is_empty() ? "" : " ", property_name));
     };
 
-    switch (kind()) {
-    case ClassMethod::Kind::Method:
-        TRY(target.define_property_or_throw(property_key, { .value = method_value, .writable = true, .enumerable = false, .configurable = true }));
-        break;
-    case ClassMethod::Kind::Getter:
-        set_function_name("get");
-        TRY(target.define_property_or_throw(property_key, { .get = &method_function, .enumerable = true, .configurable = true }));
-        break;
-    case ClassMethod::Kind::Setter:
-        set_function_name("set");
-        TRY(target.define_property_or_throw(property_key, { .set = &method_function, .enumerable = true, .configurable = true }));
-        break;
-    default:
-        VERIFY_NOT_REACHED();
-    }
-    // FIXME: Return PrivateElement for private methods
+    if (property_key.has<PropertyName>()) {
+        auto& property_name = property_key.get<PropertyName>();
+        switch (kind()) {
+        case ClassMethod::Kind::Method:
+            set_function_name();
+            TRY(target.define_property_or_throw(property_name, { .value = method_value, .writable = true, .enumerable = false, .configurable = true }));
+            break;
+        case ClassMethod::Kind::Getter:
+            set_function_name("get");
+            TRY(target.define_property_or_throw(property_name, { .get = &method_function, .enumerable = true, .configurable = true }));
+            break;
+        case ClassMethod::Kind::Setter:
+            set_function_name("set");
+            TRY(target.define_property_or_throw(property_name, { .set = &method_function, .enumerable = true, .configurable = true }));
+            break;
+        default:
+            VERIFY_NOT_REACHED();
+        }
 
-    return ClassValue { normal_completion({}) };
+        return ClassValue { normal_completion({}) };
+    } else {
+        auto& private_name = property_key.get<PrivateName>();
+        switch (kind()) {
+        case Kind::Method:
+            set_function_name();
+            return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Method, method_value } };
+        case Kind::Getter:
+            set_function_name("get");
+            return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Accessor, Accessor::create(interpreter.vm(), &method_function, nullptr) } };
+        case Kind::Setter:
+            set_function_name("set");
+            return ClassValue { PrivateElement { private_name, PrivateElement::Kind::Accessor, Accessor::create(interpreter.vm(), nullptr, &method_function) } };
+        default:
+            VERIFY_NOT_REACHED();
+        }
+    }
 }
 
 // 15.7.10 Runtime Semantics: ClassFieldDefinitionEvaluation, https://tc39.es/ecma262/#sec-runtime-semantics-classfielddefinitionevaluation
@@ -1232,8 +1268,14 @@ ThrowCompletionOr<ClassElement::ClassValue> ClassField::class_element_evaluation
     if (m_initializer) {
         auto copy_initializer = m_initializer;
         auto body = create_ast_node<ExpressionStatement>(m_initializer->source_range(), copy_initializer.release_nonnull());
+        auto name = property_key.visit(
+            [&](PropertyName const& property_name) -> String {
+                return property_name.is_number() ? property_name.to_string() : property_name.to_string_or_symbol().to_display_string();
+            },
+            [&](PrivateName const& private_name) -> String {
+                return private_name.description;
+            });
         // FIXME: A potential optimization is not creating the functions here since these are never directly accessible.
-        auto name = property_key.is_number() ? property_key.to_string() : property_key.to_string_or_symbol().to_display_string();
         initializer = ECMAScriptFunctionObject::create(interpreter.global_object(), name, *body, {}, 0, interpreter.lexical_environment(), interpreter.vm().running_execution_context().private_environment, FunctionKind::Regular, false, false);
         initializer->set_home_object(&target);
     }
@@ -1246,6 +1288,23 @@ ThrowCompletionOr<ClassElement::ClassValue> ClassField::class_element_evaluation
     };
 }
 
+static Optional<FlyString> nullopt_or_private_identifier_description(Expression const& expression)
+{
+    if (is<PrivateIdentifier>(expression))
+        return static_cast<PrivateIdentifier const&>(expression).string();
+    return {};
+}
+
+Optional<FlyString> ClassField::private_bound_identifier() const
+{
+    return nullopt_or_private_identifier_description(*m_key);
+}
+
+Optional<FlyString> ClassMethod::private_bound_identifier() const
+{
+    return nullopt_or_private_identifier_description(*m_key);
+}
+
 // 15.7.11 Runtime Semantics: ClassStaticBlockDefinitionEvaluation, https://tc39.es/ecma262/#sec-runtime-semantics-classstaticblockdefinitionevaluation
 ThrowCompletionOr<ClassElement::ClassValue> StaticInitializer::class_element_evaluation(Interpreter& interpreter, GlobalObject& global_object, Object& home_object) const
 {
@@ -1291,8 +1350,6 @@ Value ClassDeclaration::execute(Interpreter& interpreter, GlobalObject& global_o
 // 15.7.14 Runtime Semantics: ClassDefinitionEvaluation, https://tc39.es/ecma262/#sec-runtime-semantics-classdefinitionevaluation
 ThrowCompletionOr<Value> ClassExpression::class_definition_evaluation(Interpreter& interpreter, GlobalObject& global_object, FlyString const& binding_name, FlyString const& class_name) const
 {
-    // FIXME: Clean up this mix of "spec", "somewhat spec", and "not spec at all".
-
     auto& vm = interpreter.vm();
     auto* environment = vm.lexical_environment();
     VERIFY(environment);
@@ -1309,7 +1366,11 @@ ThrowCompletionOr<Value> ClassExpression::class_definition_evaluation(Interprete
     auto* outer_private_environment = vm.running_execution_context().private_environment;
     auto* class_private_environment = new_private_environment(vm, outer_private_environment);
 
-    // FIXME: Append names to private environment
+    for (auto const& element : m_elements) {
+        auto opt_private_name = element.private_bound_identifier();
+        if (opt_private_name.has_value())
+            class_private_environment->add_private_name({}, opt_private_name.release_value());
+    }
 
     auto* proto_parent = vm.current_realm()->global_object().object_prototype();
 
@@ -1385,16 +1446,38 @@ ThrowCompletionOr<Value> ClassExpression::class_definition_evaluation(Interprete
 
     using StaticElement = Variant<ClassElement::ClassFieldDefinition, ECMAScriptFunctionObject*>;
 
+    Vector<PrivateElement> static_private_methods;
+    Vector<PrivateElement> instance_private_methods;
     Vector<ClassElement::ClassFieldDefinition> instance_fields;
     Vector<StaticElement> static_elements;
 
-    // FIXME: Do things with private methods, and fields here
     for (auto const& element : m_elements) {
         // Note: All ClassElementEvaluation start with evaluating the name (or we fake it).
         auto element_value = TRY(element.class_element_evaluation(interpreter, global_object, element.is_static() ? *class_constructor : *prototype));
-        // FIXME: If element is a private element
 
-        if (auto* class_field_definition_ptr = element_value.get_pointer<ClassElement::ClassFieldDefinition>()) {
+        if (element_value.has<PrivateElement>()) {
+            auto& container = element.is_static() ? static_private_methods : instance_private_methods;
+
+            auto& private_element = element_value.get<PrivateElement>();
+
+            auto added_to_existing = false;
+            // FIXME: We can skip this loop in most cases.
+            for (auto& existing : container) {
+                if (existing.key == private_element.key) {
+                    VERIFY(existing.kind == PrivateElement::Kind::Accessor);
+                    VERIFY(private_element.kind == PrivateElement::Kind::Accessor);
+                    auto& accessor = private_element.value.as_accessor();
+                    if (!accessor.getter())
+                        existing.value.as_accessor().set_setter(accessor.setter());
+                    else
+                        existing.value.as_accessor().set_getter(accessor.getter());
+                    added_to_existing = true;
+                }
+            }
+
+            if (!added_to_existing)
+                container.append(move(element_value.get<PrivateElement>()));
+        } else if (auto* class_field_definition_ptr = element_value.get_pointer<ClassElement::ClassFieldDefinition>()) {
             if (element.is_static())
                 static_elements.append(move(*class_field_definition_ptr));
             else
@@ -1414,19 +1497,19 @@ ThrowCompletionOr<Value> ClassExpression::class_definition_evaluation(Interprete
     if (!binding_name.is_null())
         MUST(class_scope->initialize_binding(global_object, binding_name, class_constructor));
 
-    // FIXME: Set the [[PrivateMethods]]
     for (auto& field : instance_fields)
         class_constructor->add_field(field.name, field.initializer);
 
+    for (auto& private_method : instance_private_methods)
+        class_constructor->add_private_method(private_method);
+
+    for (auto& method : static_private_methods)
+        class_constructor->private_method_or_accessor_add(move(method));
+
     for (auto& element : static_elements) {
         TRY(element.visit(
             [&](ClassElement::ClassFieldDefinition const& field) -> ThrowCompletionOr<void> {
-                Value field_value = js_undefined();
-                if (field.initializer)
-                    field_value = TRY(call(global_object, field.initializer, class_constructor_value));
-
-                TRY(class_constructor->create_data_property_or_throw(field.name, field_value));
-                return {};
+                return TRY(class_constructor->define_field(field.name, field.initializer));
             },
             [&](ECMAScriptFunctionObject* static_block_function) -> ThrowCompletionOr<void> {
                 // We discard any value returned here.
@@ -1979,6 +2062,19 @@ void Identifier::dump(int indent) const
     outln("Identifier \"{}\"", m_string);
 }
 
+Value PrivateIdentifier::execute(Interpreter&, GlobalObject&) const
+{
+    // Note: This should be handled by either the member expression this is part of
+    //       or the binary expression in the case of `#foo in bar`.
+    VERIFY_NOT_REACHED();
+}
+
+void PrivateIdentifier::dump(int indent) const
+{
+    print_indent(indent);
+    outln("PrivateIdentifier \"{}\"", m_string);
+}
+
 void SpreadExpression::dump(int indent) const
 {
     ASTNode::dump(indent);
@@ -2527,6 +2623,17 @@ Value MemberExpression::execute(Interpreter& interpreter, GlobalObject& global_o
     return reference.get_value(global_object);
 }
 
+bool MemberExpression::ends_in_private_name() const
+{
+    if (is_computed())
+        return false;
+    if (is<PrivateIdentifier>(*m_property))
+        return true;
+    if (is<MemberExpression>(*m_property))
+        return static_cast<MemberExpression const&>(*m_property).ends_in_private_name();
+    return false;
+}
+
 void OptionalChain::dump(int indent) const
 {
     print_indent(indent);

Userland/Libraries/LibJS/AST.h

@@ -1000,6 +1000,23 @@ private:
     mutable Optional<EnvironmentCoordinate> m_cached_environment_coordinate;
 };
 
+class PrivateIdentifier final : public Expression {
+public:
+    explicit PrivateIdentifier(SourceRange source_range, FlyString string)
+        : Expression(source_range)
+        , m_string(move(string))
+    {
+    }
+
+    FlyString const& string() const { return m_string; }
+
+    virtual Value execute(Interpreter&, GlobalObject&) const override;
+    virtual void dump(int indent) const override;
+
+private:
+    FlyString m_string;
+};
+
 class ClassElement : public ASTNode {
 public:
     ClassElement(SourceRange source_range, bool is_static)
@@ -1019,15 +1036,19 @@ public:
     virtual ElementKind class_element_kind() const = 0;
     bool is_static() const { return m_is_static; }
 
+    using ClassElementName = Variant<PropertyName, PrivateName>;
+
     struct ClassFieldDefinition {
-        PropertyName name;
+        ClassElementName name;
         ECMAScriptFunctionObject* initializer { nullptr };
     };
 
     // We use the Completion also as a ClassStaticBlockDefinition Record.
-    using ClassValue = Variant<ClassFieldDefinition, Completion>;
+    using ClassValue = Variant<ClassFieldDefinition, Completion, PrivateElement>;
     virtual ThrowCompletionOr<ClassValue> class_element_evaluation(Interpreter&, GlobalObject&, Object& home_object) const = 0;
 
+    virtual Optional<FlyString> private_bound_identifier() const { return {}; };
+
 private:
     bool m_is_static { false };
 };
@@ -1054,6 +1075,7 @@ public:
 
     virtual void dump(int indent) const override;
     virtual ThrowCompletionOr<ClassValue> class_element_evaluation(Interpreter&, GlobalObject&, Object& home_object) const override;
+    virtual Optional<FlyString> private_bound_identifier() const override;
 
 private:
     NonnullRefPtr<Expression> m_key;
@@ -1078,6 +1100,7 @@ public:
 
     virtual void dump(int indent) const override;
     virtual ThrowCompletionOr<ClassValue> class_element_evaluation(Interpreter& interpreter, GlobalObject& object, Object& home_object) const override;
+    virtual Optional<FlyString> private_bound_identifier() const override;
 
 private:
     NonnullRefPtr<Expression> m_key;
@@ -1536,6 +1559,8 @@ public:
 
     String to_string_approximation() const;
 
+    bool ends_in_private_name() const;
+
 private:
     virtual bool is_member_expression() const override { return true; }
 

Userland/Libraries/LibJS/Parser.cpp

@@ -850,6 +850,7 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
     NonnullRefPtrVector<ClassElement> elements;
     RefPtr<Expression> super_class;
     RefPtr<FunctionExpression> constructor;
+    HashTable<FlyString> found_private_names;
 
     String class_name = expect_class_name || match_identifier() || match(TokenType::Yield) || match(TokenType::Await)
         ? consume_identifier_reference().value().to_string()
@@ -881,6 +882,13 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
 
     consume(TokenType::CurlyOpen);
 
+    HashTable<StringView> referenced_private_names;
+    HashTable<StringView>* outer_referenced_private_names = m_state.referenced_private_names;
+    m_state.referenced_private_names = &referenced_private_names;
+    ScopeGuard restore_private_name_table = [&] {
+        m_state.referenced_private_names = outer_referenced_private_names;
+    };
+
     while (!done() && !match(TokenType::CurlyClose)) {
         RefPtr<Expression> property_key;
         bool is_static = false;
@@ -899,7 +907,7 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
         }
 
         StringView name;
-        if (match_property_key()) {
+        if (match_property_key() || match(TokenType::PrivateIdentifier)) {
             if (!is_generator && m_state.current_token.original_value() == "static"sv) {
                 if (match(TokenType::Identifier)) {
                     consume();
@@ -923,12 +931,49 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
                 }
             }
 
-            if (match_property_key()) {
+            if (match_property_key() || match(TokenType::PrivateIdentifier)) {
                 switch (m_state.current_token.type()) {
                 case TokenType::Identifier:
                     name = consume().value();
                     property_key = create_ast_node<StringLiteral>({ m_state.current_token.filename(), rule_start.position(), position() }, name);
                     break;
+                case TokenType::PrivateIdentifier:
+                    name = consume().value();
+                    if (name == "#constructor")
+                        syntax_error("Private property with name '#constructor' is not allowed");
+
+                    if (method_kind != ClassMethod::Kind::Method) {
+                        // It is a Syntax Error if PrivateBoundIdentifiers of ClassElementList contains any duplicate entries,
+                        //   unless the name is used once for a getter and once for a setter and in no other entries,
+                        //   and the getter and setter are either both static or both non-static.
+
+                        for (auto& element : elements) {
+                            auto private_name = element.private_bound_identifier();
+                            if (!private_name.has_value() || private_name.value() != name)
+                                continue;
+
+                            if (element.class_element_kind() != ClassElement::ElementKind::Method
+                                || element.is_static() != is_static) {
+                                syntax_error(String::formatted("Duplicate private field or method named '{}'", name));
+                                break;
+                            }
+
+                            VERIFY(is<ClassMethod>(element));
+                            auto& class_method_element = static_cast<ClassMethod const&>(element);
+
+                            if (class_method_element.kind() == ClassMethod::Kind::Method || class_method_element.kind() == method_kind) {
+                                syntax_error(String::formatted("Duplicate private field or method named '{}'", name));
+                                break;
+                            }
+                        }
+
+                        found_private_names.set(name);
+                    } else if (found_private_names.set(name) != AK::HashSetResult::InsertedNewEntry) {
+                        syntax_error(String::formatted("Duplicate private field or method named '{}'", name));
+                    }
+
+                    property_key = create_ast_node<PrivateIdentifier>({ m_state.current_token.filename(), rule_start.position(), position() }, name);
+                    break;
                 case TokenType::StringLiteral: {
                     auto string_literal = parse_string_literal(consume());
                     name = string_literal->value();
@@ -1064,6 +1109,16 @@ NonnullRefPtr<ClassExpression> Parser::parse_class_expression(bool expect_class_
         }
     }
 
+    // We could be in a subclass defined within the main class so must move all non declared private names to outer.
+    for (auto& private_name : referenced_private_names) {
+        if (found_private_names.contains(private_name))
+            continue;
+        if (outer_referenced_private_names)
+            outer_referenced_private_names->set(private_name);
+        else // FIXME: Make these error appear in the appropriate places.
+            syntax_error(String::formatted("Reference to undeclared private field or method '{}'", private_name));
+    }
+
     return create_ast_node<ClassExpression>({ m_state.current_token.filename(), rule_start.position(), position() }, move(class_name), move(constructor), move(super_class), move(elements));
 }
 
@@ -1264,6 +1319,11 @@ NonnullRefPtr<Expression> Parser::parse_unary_prefixed_expression()
         if (is<Identifier>(*rhs) && m_state.strict_mode) {
             syntax_error("Delete of an unqualified identifier in strict mode.", rhs_start);
         }
+        if (is<MemberExpression>(*rhs)) {
+            auto& member_expression = static_cast<MemberExpression const&>(*rhs);
+            if (member_expression.ends_in_private_name())
+                syntax_error("Private fields cannot be deleted");
+        }
         return create_ast_node<UnaryExpression>({ m_state.current_token.filename(), rule_start.position(), position() }, UnaryOp::Delete, move(rhs));
     }
     default:
@@ -1711,8 +1771,17 @@ NonnullRefPtr<Expression> Parser::parse_secondary_expression(NonnullRefPtr<Expre
         return parse_assignment_expression(AssignmentOp::Assignment, move(lhs), min_precedence, associativity);
     case TokenType::Period:
         consume();
-        if (!match_identifier_name())
+        if (match(TokenType::PrivateIdentifier)) {
+            if (!is_private_identifier_valid())
+                syntax_error(String::formatted("Reference to undeclared private field or method '{}'", m_state.current_token.value()));
+            else if (is<SuperExpression>(*lhs))
+                syntax_error(String::formatted("Cannot access private field or method '{}' on super", m_state.current_token.value()));
+
+            return create_ast_node<MemberExpression>({ m_state.current_token.filename(), rule_start.position(), position() }, move(lhs), create_ast_node<PrivateIdentifier>({ m_state.current_token.filename(), rule_start.position(), position() }, consume().value()));
+        } else if (!match_identifier_name()) {
             expected("IdentifierName");
+        }
+
         return create_ast_node<MemberExpression>({ m_state.current_token.filename(), rule_start.position(), position() }, move(lhs), create_ast_node<Identifier>({ m_state.current_token.filename(), rule_start.position(), position() }, consume().value()));
     case TokenType::BracketOpen: {
         consume(TokenType::BracketOpen);
@@ -1780,6 +1849,17 @@ NonnullRefPtr<Expression> Parser::parse_secondary_expression(NonnullRefPtr<Expre
     }
 }
 
+bool Parser::is_private_identifier_valid() const
+{
+    VERIFY(match(TokenType::PrivateIdentifier));
+    if (!m_state.referenced_private_names)
+        return false;
+
+    // We might not have hit the declaration yet so class will check this in the end
+    m_state.referenced_private_names->set(m_state.current_token.value());
+    return true;
+}
+
 RefPtr<BindingPattern> Parser::synthesize_binding_pattern(Expression const& expression)
 {
     VERIFY(is<ArrayExpression>(expression) || is<ObjectExpression>(expression));
@@ -3042,6 +3122,7 @@ bool Parser::match_expression() const
         || type == TokenType::TemplateLiteralStart
         || type == TokenType::NullLiteral
         || match_identifier()
+        || (type == TokenType::PrivateIdentifier && next_token().type() == TokenType::In)
         || type == TokenType::Await
         || type == TokenType::New
         || type == TokenType::Class

Userland/Libraries/LibJS/Parser.h

@@ -178,6 +178,7 @@ private:
     bool match_identifier() const;
     bool match_identifier_name() const;
     bool match_property_key() const;
+    bool is_private_identifier_valid() const;
     bool match(TokenType type) const;
     bool done() const;
     void expected(const char* what);
@@ -242,6 +243,8 @@ private:
         ScopePusher* current_scope_pusher { nullptr };
 
         HashMap<StringView, Optional<Position>> labels_in_scope;
+        HashTable<StringView>* referenced_private_names { nullptr };
+
         bool strict_mode { false };
         bool allow_super_property_lookup { false };
         bool allow_super_constructor_call { false };

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -276,8 +276,8 @@ void ECMAScriptFunctionObject::visit_edges(Visitor& visitor)
     visitor.visit(m_home_object);
 
     for (auto& field : m_fields) {
-        if (field.name.is_symbol())
-            visitor.visit(field.name.as_symbol());
+        if (auto* property_name_ptr = field.name.get_pointer<PropertyName>(); property_name_ptr && property_name_ptr->is_symbol())
+            visitor.visit(property_name_ptr->as_symbol());
 
         visitor.visit(field.initializer);
     }
@@ -729,17 +729,9 @@ void ECMAScriptFunctionObject::set_name(const FlyString& name)
     VERIFY(success);
 }
 
-// 7.3.31 DefineField ( receiver, fieldRecord ), https://tc39.es/ecma262/#sec-definefield
-void ECMAScriptFunctionObject::InstanceField::define_field(VM& vm, Object& receiver) const
+void ECMAScriptFunctionObject::add_field(ClassElement::ClassElementName property_key, ECMAScriptFunctionObject* initializer)
 {
-    Value init_value = js_undefined();
-    if (initializer) {
-        auto init_value_or_error = vm.call(*initializer, receiver.value_of());
-        if (init_value_or_error.is_error())
-            return;
-        init_value = init_value_or_error.release_value();
-    }
-    (void)receiver.create_data_property_or_throw(name, init_value);
+    m_fields.empend(property_key, initializer);
 }
 
 }

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h

@@ -59,14 +59,15 @@ public:
     void set_home_object(Object* home_object) { m_home_object = home_object; }
 
     struct InstanceField {
-        PropertyName name;
+        Variant<PropertyName, PrivateName> name;
         ECMAScriptFunctionObject* initializer { nullptr };
-
-        void define_field(VM& vm, Object& receiver) const;
     };
 
     Vector<InstanceField> const& fields() const { return m_fields; }
-    void add_field(PropertyName property_key, ECMAScriptFunctionObject* initializer) { m_fields.empend(property_key, initializer); }
+    void add_field(Variant<PropertyName, PrivateName> property_key, ECMAScriptFunctionObject* initializer);
+
+    Vector<PrivateElement> const& private_methods() const { return m_private_methods; }
+    void add_private_method(PrivateElement method) { m_private_methods.append(move(method)); };
 
     // This is for IsSimpleParameterList (static semantics)
     bool has_simple_parameter_list() const { return m_has_simple_parameter_list; }
@@ -97,6 +98,7 @@ private:
     bool m_strict { false };                                      // [[Strict]]
     Object* m_home_object { nullptr };                            // [[HomeObject]]
     Vector<InstanceField> m_fields;                               // [[Fields]]
+    Vector<PrivateElement> m_private_methods;                     // [[PrivateMethods]]
     bool m_is_class_constructor { false };                        // [[IsClassConstructor]]
 
     FlyString m_name;

Userland/Libraries/LibJS/Runtime/PrivateEnvironment.cpp

@@ -34,8 +34,9 @@ PrivateName PrivateEnvironment::resolve_private_identifier(FlyString const& iden
 
 void PrivateEnvironment::add_private_name(Badge<ClassExpression>, FlyString description)
 {
-    // FIXME: there is a exception for getter setter pairs.
-    VERIFY(find_private_name(description).is_end());
+    if (!find_private_name(description).is_end())
+        return;
+
     m_private_names.empend(m_unique_id, move(description));
 }
 

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -478,11 +478,11 @@ Reference VM::resolve_binding(FlyString const& name, Environment* environment)
 // 7.3.32 InitializeInstanceElements ( O, constructor ), https://tc39.es/ecma262/#sec-initializeinstanceelements
 ThrowCompletionOr<void> VM::initialize_instance_elements(Object& object, ECMAScriptFunctionObject& constructor)
 {
-    for (auto& field : constructor.fields()) {
-        field.define_field(*this, object);
-        if (auto* exception = this->exception())
-            return JS::throw_completion(exception->value());
-    }
+    for (auto& method : constructor.private_methods())
+        TRY(object.private_method_or_accessor_add(method));
+
+    for (auto& field : constructor.fields())
+        TRY(object.define_field(field.name, field.initializer));
     return {};
 }
 

Userland/Libraries/LibJS/Tests/classes/class-private-fields.js

@@ -0,0 +1,88 @@
+test("basic functionality", () => {
+    class A {
+        #number = 3;
+
+        getNumber() {
+            return this.#number;
+        }
+
+        #string = "foo";
+
+        getString() {
+            return this.#string;
+        }
+
+        #uninitialized;
+
+        getUninitialized() {
+            return this.#uninitialized;
+        }
+    }
+
+    const a = new A();
+    expect(a.getNumber()).toBe(3);
+    expect(a.getString()).toBe("foo");
+    expect(a.getUninitialized()).toBeUndefined();
+
+    expect("a.#number").not.toEval();
+    expect("a.#string").not.toEval();
+    expect("a.#uninitialized").not.toEval();
+});
+
+test("initializer has correct this value", () => {
+    class A {
+        #thisVal = this;
+
+        getThisVal() {
+            return this.#thisVal;
+        }
+
+        #thisName = this.#thisVal;
+
+        getThisName() {
+            return this.#thisName;
+        }
+    }
+
+    const a = new A();
+    expect(a.getThisVal()).toBe(a);
+    expect(a.getThisName()).toBe(a);
+});
+
+test("static fields", () => {
+    class A {
+        static #simple = 1;
+
+        static getStaticSimple() {
+            return this.#simple;
+        }
+
+        static #thisVal = this;
+        static #thisName = this.name;
+        static #thisVal2 = this.#thisVal;
+
+        static getThisVal() {
+            return this.#thisVal;
+        }
+
+        static getThisName() {
+            return this.#thisName;
+        }
+
+        static getThisVal2() {
+            return this.#thisVal2;
+        }
+    }
+
+    expect(A.getStaticSimple()).toBe(1);
+
+    expect(A.getThisVal()).toBe(A);
+    expect(A.getThisName()).toBe("A");
+    expect(A.getThisVal2()).toBe(A);
+
+    expect("A.#simple").not.toEval();
+});
+
+test("cannot have static and non static field with the same description", () => {
+    expect("class A { static #simple; #simple; }").not.toEval();
+});