LibJS: Remove default constructor of PropertyKey

This constructor was creating an "invalid" `PropertyKey`, but every
code path constructing such a `PropertyKey` was either never used or
immediately `VERIFY`ing that the `PropertyKey` was valid.

The `VERIFY` call has been moved to the `PropertyKey::from_value(...)`
call, and the array/object literal spreading code could be refactored
to not take a `PropertyKey` but creates dummy values for now.

The default constructor for `Reference` has similairly be deleted,
because it was never used.

Userland/Libraries/LibJS/Bytecode/Interpreter.cpp

@@ -1362,7 +1362,7 @@ inline ThrowCompletionOr<void> put_by_value(VM& vm, Value base, Optional<Depreca
         }
     }
 
-    auto property_key = kind != Op::PropertyKind::Spread ? TRY(property_key_value.to_property_key(vm)) : PropertyKey {};
+    auto property_key = kind != Op::PropertyKind::Spread ? TRY(property_key_value.to_property_key(vm)) : PropertyKey { 0 };
     TRY(put_by_property_key(vm, base, base, value, base_identifier, property_key, kind));
     return {};
 }
@@ -2807,7 +2807,7 @@ ThrowCompletionOr<void> PutByValueWithThis::execute_impl(Bytecode::Interpreter&
     auto& vm = interpreter.vm();
     auto value = interpreter.get(m_src);
     auto base = interpreter.get(m_base);
-    auto property_key = m_kind != PropertyKind::Spread ? TRY(interpreter.get(m_property).to_property_key(vm)) : PropertyKey {};
+    auto property_key = m_kind != PropertyKind::Spread ? TRY(interpreter.get(m_property).to_property_key(vm)) : PropertyKey { 0 };
     TRY(put_by_property_key(vm, base, interpreter.get(m_this_value), value, {}, property_key, m_kind));
     return {};
 }

Userland/Libraries/LibJS/Runtime/Object.cpp

@@ -1448,18 +1448,13 @@ ThrowCompletionOr<Value> Object::ordinary_to_primitive(Value::PreferredType pref
 
     auto& vm = this->vm();
 
-    AK::Array<PropertyKey, 2> method_names;
-
-    // 1. If hint is string, then
-    if (preferred_type == Value::PreferredType::String) {
+    AK::Array<PropertyKey, 2> method_names = (preferred_type == Value::PreferredType::String)
+        // 1. If hint is string, then
         // a. Let methodNames be « "toString", "valueOf" ».
-        method_names = { vm.names.toString, vm.names.valueOf };
-    }
-    // 2. Else,
-    else {
+        ? AK::Array { vm.names.toString, vm.names.valueOf }
+        // 2. Else,
         // a. Let methodNames be « "valueOf", "toString" ».
-        method_names = { vm.names.valueOf, vm.names.toString };
-    }
+        : AK::Array { vm.names.valueOf, vm.names.toString };
 
     // 3. For each element name of methodNames, do
     for (auto& method_name : method_names) {

Userland/Libraries/LibJS/Runtime/PropertyKey.h

@@ -30,8 +30,7 @@ public:
 
     static ThrowCompletionOr<PropertyKey> from_value(VM& vm, Value value)
     {
-        if (value.is_empty())
-            return PropertyKey {};
+        VERIFY(!value.is_empty());
         if (value.is_symbol())
             return PropertyKey { value.as_symbol() };
         if (value.is_integral_number() && value.as_double() >= 0 && value.as_double() < NumericLimits<u32>::max())
@@ -39,7 +38,7 @@ public:
         return TRY(value.to_byte_string(vm));
     }
 
-    PropertyKey() = default;
+    PropertyKey() = delete;
 
     template<Integral T>
     PropertyKey(T index)

Userland/Libraries/LibJS/Runtime/Reference.cpp

@@ -31,7 +31,7 @@ ThrowCompletionOr<void> Reference::put_value(VM& vm, Value value)
         auto& global_object = vm.get_global_object();
 
         // c. Perform ? Set(globalObj, V.[[ReferencedName]], W, false).
-        TRY(global_object.set(m_name, value, Object::ShouldThrowExceptions::No));
+        TRY(global_object.set(name(), value, Object::ShouldThrowExceptions::No));
 
         // Return unused.
         return {};
@@ -45,15 +45,15 @@ ThrowCompletionOr<void> Reference::put_value(VM& vm, Value value)
         // b. If IsPrivateReference(V) is true, then
         if (is_private_reference()) {
             // i. Return ? PrivateSet(baseObj, V.[[ReferencedName]], W).
-            return base_obj->private_set(m_private_name, value);
+            return base_obj->private_set(private_name(), value);
         }
 
         // c. Let succeeded be ? baseObj.[[Set]](V.[[ReferencedName]], W, GetThisValue(V)).
-        auto succeeded = TRY(base_obj->internal_set(m_name, value, get_this_value()));
+        auto succeeded = TRY(base_obj->internal_set(name(), value, get_this_value()));
 
         // d. If succeeded is false and V.[[Strict]] is true, throw a TypeError exception.
         if (!succeeded && m_strict)
-            return vm.throw_completion<TypeError>(ErrorType::ReferenceNullishSetProperty, m_name, m_base_value.to_string_without_side_effects());
+            return vm.throw_completion<TypeError>(ErrorType::ReferenceNullishSetProperty, name(), m_base_value.to_string_without_side_effects());
 
         // e. Return unused.
         return {};
@@ -70,15 +70,15 @@ ThrowCompletionOr<void> Reference::put_value(VM& vm, Value value)
     if (m_environment_coordinate.has_value())
         return static_cast<DeclarativeEnvironment*>(m_base_environment)->set_mutable_binding_direct(vm, m_environment_coordinate->index, value, m_strict);
     else
-        return m_base_environment->set_mutable_binding(vm, m_name.as_string(), value, m_strict);
+        return m_base_environment->set_mutable_binding(vm, name().as_string(), value, m_strict);
 }
 
 Completion Reference::throw_reference_error(VM& vm) const
 {
-    if (!m_name.is_valid())
+    if (is_private_reference())
         return vm.throw_completion<ReferenceError>(ErrorType::ReferenceUnresolvable);
     else
-        return vm.throw_completion<ReferenceError>(ErrorType::UnknownIdentifier, m_name.to_string_or_symbol().to_display_string());
+        return vm.throw_completion<ReferenceError>(ErrorType::UnknownIdentifier, name().to_string_or_symbol().to_display_string());
 }
 
 // 6.2.4.5 GetValue ( V ), https://tc39.es/ecma262/#sec-getvalue
@@ -108,13 +108,13 @@ ThrowCompletionOr<Value> Reference::get_value(VM& vm) const
             auto base_obj = TRY(m_base_value.to_object(vm));
 
             // i. Return ? PrivateGet(baseObj, V.[[ReferencedName]]).
-            return base_obj->private_get(m_private_name);
+            return base_obj->private_get(private_name());
         }
 
         // OPTIMIZATION: For various primitives we can avoid actually creating a new object for them.
         GCPtr<Object> base_obj;
         if (m_base_value.is_string()) {
-            auto string_value = TRY(m_base_value.as_string().get(vm, m_name));
+            auto string_value = TRY(m_base_value.as_string().get(vm, name()));
             if (string_value.has_value())
                 return *string_value;
             base_obj = realm.intrinsics().string_prototype();
@@ -130,7 +130,7 @@ ThrowCompletionOr<Value> Reference::get_value(VM& vm) const
             base_obj = TRY(m_base_value.to_object(vm));
 
         // c. Return ? baseObj.[[Get]](V.[[ReferencedName]], GetThisValue(V)).
-        return base_obj->internal_get(m_name, get_this_value());
+        return base_obj->internal_get(name(), get_this_value());
     }
 
     // 5. Else,
@@ -143,7 +143,7 @@ ThrowCompletionOr<Value> Reference::get_value(VM& vm) const
     // c. Return ? base.GetBindingValue(V.[[ReferencedName]], V.[[Strict]]) (see 9.1).
     if (m_environment_coordinate.has_value())
         return static_cast<DeclarativeEnvironment*>(m_base_environment)->get_binding_value_direct(vm, m_environment_coordinate->index);
-    return m_base_environment->get_binding_value(vm, m_name.as_string(), m_strict);
+    return m_base_environment->get_binding_value(vm, name().as_string(), m_strict);
 }
 
 // 13.5.1.2 Runtime Semantics: Evaluation, https://tc39.es/ecma262/#sec-delete-operator-runtime-semantics-evaluation
@@ -178,11 +178,11 @@ ThrowCompletionOr<bool> Reference::delete_(VM& vm)
         auto base_obj = TRY(m_base_value.to_object(vm));
 
         // d. Let deleteStatus be ? baseObj.[[Delete]](ref.[[ReferencedName]]).
-        bool delete_status = TRY(base_obj->internal_delete(m_name));
+        bool delete_status = TRY(base_obj->internal_delete(name()));
 
         // e. If deleteStatus is false and ref.[[Strict]] is true, throw a TypeError exception.
         if (!delete_status && m_strict)
-            return vm.throw_completion<TypeError>(ErrorType::ReferenceNullishDeleteProperty, m_name, m_base_value.to_string_without_side_effects());
+            return vm.throw_completion<TypeError>(ErrorType::ReferenceNullishDeleteProperty, name(), m_base_value.to_string_without_side_effects());
 
         // f. Return deleteStatus.
         return delete_status;
@@ -195,7 +195,7 @@ ThrowCompletionOr<bool> Reference::delete_(VM& vm)
     VERIFY(m_base_type == BaseType::Environment);
 
     //    c. Return ? base.DeleteBinding(ref.[[ReferencedName]]).
-    return m_base_environment->delete_binding(vm, m_name.as_string());
+    return m_base_environment->delete_binding(vm, name().as_string());
 }
 
 // 6.2.4.8 InitializeReferencedBinding ( V, W ), https://tc39.es/ecma262/#sec-object.prototype.hasownproperty
@@ -204,7 +204,7 @@ ThrowCompletionOr<void> Reference::initialize_referenced_binding(VM& vm, Value v
 {
     VERIFY(!is_unresolvable());
     VERIFY(m_base_type == BaseType::Environment);
-    return m_base_environment->initialize_binding(vm, m_name.as_string(), value, hint);
+    return m_base_environment->initialize_binding(vm, name().as_string(), value, hint);
 }
 
 // 6.2.4.9 MakePrivateReference ( baseValue, privateIdentifier ), https://tc39.es/ecma262/#sec-makeprivatereference

Userland/Libraries/LibJS/Runtime/Reference.h

@@ -23,7 +23,6 @@ public:
         Environment,
     };
 
-    Reference() = default;
     Reference(BaseType type, PropertyKey name, bool strict)
         : m_base_type(type)
         , m_name(move(name))
@@ -52,10 +51,8 @@ public:
     Reference(Value base, PrivateName name)
         : m_base_type(BaseType::Value)
         , m_base_value(base)
-        , m_this_value(Value {})
+        , m_name(move(name))
         , m_strict(true)
-        , m_is_private(true)
-        , m_private_name(move(name))
     {
     }
 
@@ -71,7 +68,8 @@ public:
         return *m_base_environment;
     }
 
-    PropertyKey const& name() const { return m_name; }
+    PropertyKey const& name() const { return m_name.get<PropertyKey>(); }
+    PrivateName const& private_name() const { return m_name.get<PrivateName>(); }
     bool is_strict() const { return m_strict; }
 
     // 6.2.4.2 IsUnresolvableReference ( V ), https://tc39.es/ecma262/#sec-isunresolvablereference
@@ -105,7 +103,7 @@ public:
     // 6.2.4.4 IsPrivateReference ( V ), https://tc39.es/ecma262/#sec-isprivatereference
     bool is_private_reference() const
     {
-        return m_is_private;
+        return m_name.has<PrivateName>();
     }
 
     // Note: Non-standard helper.
@@ -120,7 +118,7 @@ public:
     ThrowCompletionOr<Value> get_value(VM&) const;
     ThrowCompletionOr<bool> delete_(VM&);
 
-    bool is_valid_reference() const { return m_name.is_valid() || m_is_private; }
+    bool is_valid_reference() const { return true; }
 
     Optional<EnvironmentCoordinate> environment_coordinate() const { return m_environment_coordinate; }
 
@@ -132,14 +130,10 @@ private:
         Value m_base_value {};
         mutable Environment* m_base_environment;
     };
-    PropertyKey m_name;
+    Variant<PropertyKey, PrivateName> m_name;
     Value m_this_value;
     bool m_strict { false };
 
-    bool m_is_private { false };
-    // FIXME: This can (probably) be an union with m_name.
-    PrivateName m_private_name;
-
     Optional<EnvironmentCoordinate> m_environment_coordinate;
 };