LibJS: Add getter/setter support

This patch adds a GetterSetterPair object. Values can now store pointers
to objects of this type. These objects are created when using
Object.defineProperty and providing an accessor descriptor.

Libraries/LibJS/Forward.h

@@ -59,6 +59,7 @@ class DeferGC;
 class Error;
 class Exception;
 class Expression;
+class Accessor;
 class GlobalObject;
 class HandleImpl;
 class Heap;

Libraries/LibJS/Runtime/Accessor.h

@@ -0,0 +1,79 @@
+/*
+ * Copyright (c) 2020, Matthew Olsson <matthewcolsson@gmail.com>
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ *    list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ *    this list of conditions and the following disclaimer in the documentation
+ *    and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
+ * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
+ * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE
+ * FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
+ * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
+ * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
+ * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
+ * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+ * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#pragma once
+
+#include <LibJS/Runtime/Cell.h>
+#include <LibJS/Runtime/Value.h>
+
+namespace JS {
+
+class Accessor final : public Cell {
+public:
+    static Accessor* create(Interpreter& interpreter, Value getter, Value setter)
+    {
+        return interpreter.heap().allocate<Accessor>(getter, setter);
+    }
+
+    Accessor(Value getter, Value setter)
+        : m_getter(getter)
+        , m_setter(setter)
+    {
+    }
+
+    Value getter() { return m_getter; }
+    Value setter() { return m_setter; }
+
+    Value call_getter(Value this_object)
+    {
+        if (!getter().is_function())
+            return js_undefined();
+        return interpreter().call(getter().as_function(), this_object);
+    }
+
+    void call_setter(Value this_object, Value setter_value)
+    {
+        if (!setter().is_function())
+            return;
+        MarkedValueList arguments(interpreter().heap());
+        arguments.values().append(setter_value);
+        interpreter().call(setter().as_function(), this_object, move(arguments));
+    }
+
+    void visit_children(Cell::Visitor& visitor) override
+    {
+        visitor.visit(m_getter);
+        visitor.visit(m_setter);
+    }
+
+private:
+    const char* class_name() const override { return "Accessor"; };
+
+    Value m_getter;
+    Value m_setter;
+};
+
+}

Libraries/LibJS/Runtime/Object.cpp

@@ -27,6 +27,7 @@
 #include <AK/String.h>
 #include <LibJS/Heap/Heap.h>
 #include <LibJS/Interpreter.h>
+#include <LibJS/Runtime/Accessor.h>
 #include <LibJS/Runtime/Array.h>
 #include <LibJS/Runtime/Error.h>
 #include <LibJS/Runtime/GlobalObject.h>
@@ -96,6 +97,9 @@ Value Object::get_own_property(const Object& this_object, const FlyString& prope
 
     auto value_here = m_storage[metadata.value().offset];
     ASSERT(!value_here.is_empty());
+    if (value_here.is_accessor()) {
+        return value_here.as_accessor().call_getter(Value(const_cast<Object*>(this)));
+    }
     if (value_here.is_object() && value_here.as_object().is_native_property()) {
         auto& native_property = static_cast<const NativeProperty&>(value_here.as_object());
         auto& interpreter = const_cast<Object*>(this)->interpreter();
@@ -180,10 +184,16 @@ Value Object::get_own_property_descriptor(const FlyString& property_name) const
     if (interpreter().exception())
         return {};
     auto* descriptor = Object::create_empty(interpreter(), interpreter().global_object());
-    descriptor->put("value", value.value_or(js_undefined()));
+    descriptor->put("enumerable", Value((metadata.value().attributes & Attribute::Enumerable) != 0));
-    descriptor->put("writable", Value(!!(metadata.value().attributes & Attribute::Writable)));
+    descriptor->put("configurable", Value((metadata.value().attributes & Attribute::Configurable) != 0));
-    descriptor->put("enumerable", Value(!!(metadata.value().attributes & Attribute::Enumerable)));
+    if (value.is_accessor()) {
-    descriptor->put("configurable", Value(!!(metadata.value().attributes & Attribute::Configurable)));
+        auto& pair = value.as_accessor();
+        descriptor->put("get", pair.getter());
+        descriptor->put("set", pair.setter());
+    } else {
+        descriptor->put("value", value.value_or(js_undefined()));
+        descriptor->put("writable", Value((metadata.value().attributes & Attribute::Writable) != 0));
+    }
     return descriptor;
 }
 
@@ -195,19 +205,74 @@ void Object::set_shape(Shape& new_shape)
 
 bool Object::define_property(const FlyString& property_name, const Object& descriptor, bool throw_exceptions)
 {
-    auto value = descriptor.get("value");
+    bool is_accessor_property = descriptor.has_property("get") || descriptor.has_property("set");
     u8 configurable = descriptor.get("configurable").value_or(Value(false)).to_boolean() * Attribute::Configurable;
+    if (interpreter().exception())
+        return {};
     u8 enumerable = descriptor.get("enumerable").value_or(Value(false)).to_boolean() * Attribute::Enumerable;
-    u8 writable = descriptor.get("writable").value_or(Value(false)).to_boolean() * Attribute::Writable;
+    if (interpreter().exception())
-    u8 attributes = configurable | enumerable | writable;
+        return {};
+    u8 attributes = configurable | enumerable;
 
-    dbg() << "Defining new property " << property_name << " with descriptor { " << configurable << ", " << enumerable << ", " << writable << ", attributes=" << attributes << " }";
+    if (is_accessor_property) {
+        if (descriptor.has_property("value") || descriptor.has_property("writable")) {
+            if (throw_exceptions)
+                interpreter().throw_exception<TypeError>("Accessor property descriptors cannot specify a value or writable key");
+            return false;
+        }
+
+        auto getter = descriptor.get("get");
+        if (interpreter().exception())
+            return {};
+        auto setter = descriptor.get("set");
+        if (interpreter().exception())
+            return {};
+
+        if (!(getter.is_empty() || getter.is_undefined() || getter.is_function())) {
+            interpreter().throw_exception<TypeError>("Accessor descriptor's 'get' field must be a function or undefined");
+            return false;
+        }
+
+        if (!(setter.is_empty() || setter.is_undefined() || setter.is_function())) {
+            interpreter().throw_exception<TypeError>("Accessor descriptor's 'set' field must be a function or undefined");
+            return false;
+        }
+
+        // FIXME: Throw a TypeError if the setter does not take any arguments
+
+        dbg() << "Defining new property " << property_name << " with accessor descriptor { attributes=" << attributes
+              << " , getter=" << (getter.is_empty() ? "<empty>" : getter.to_string_without_side_effects())
+              << ", setter=" << (setter.is_empty() ? "<empty>" : setter.to_string_without_side_effects()) << "}";
+
+        return put_own_property(*this, property_name, attributes, Accessor::create(interpreter(), getter, setter), PutOwnPropertyMode::DefineProperty, throw_exceptions);
+    }
+
+    auto value = descriptor.get("value");
+    if (interpreter().exception())
+        return {};
+    u8 writable = descriptor.get("writable").value_or(Value(false)).to_boolean() * Attribute::Writable;
+    if (interpreter().exception())
+        return {};
+    attributes |= writable;
+
+    dbg() << "Defining new property " << property_name << " with data descriptor { attributes=" << attributes
+          << ", value=" << (value.is_empty() ? "<empty>" : value.to_string_without_side_effects()) << " }";
 
     return put_own_property(*this, property_name, attributes, value, PutOwnPropertyMode::DefineProperty, throw_exceptions);
 }
 
 bool Object::put_own_property(Object& this_object, const FlyString& property_name, u8 attributes, Value value, PutOwnPropertyMode mode, bool throw_exceptions)
 {
+    ASSERT(!(mode == PutOwnPropertyMode::Put && value.is_accessor()));
+
+    if (value.is_accessor()) {
+        auto& accessor = value.as_accessor();
+        if (accessor.getter().is_function())
+            attributes |= Attribute::HasGet;
+        if (accessor.setter().is_function())
+            attributes |= Attribute::HasSet;
+    }
+
     auto metadata = shape().lookup(property_name);
     bool new_property = !metadata.has_value();
 
@@ -231,7 +296,7 @@ bool Object::put_own_property(Object& this_object, const FlyString& property_nam
     if (!new_property && mode == PutOwnPropertyMode::DefineProperty && !(metadata.value().attributes & Attribute::Configurable) && attributes != metadata.value().attributes) {
         dbg() << "Disallow reconfig of non-configurable property";
         if (throw_exceptions)
-            interpreter().throw_exception<TypeError>(String::format("Cannot redefine property '%s'", property_name.characters()));
+            interpreter().throw_exception<TypeError>(String::format("Cannot change attributes of non-configurable property '%s'", property_name.characters()));
         return false;
     }
 
@@ -246,7 +311,8 @@ bool Object::put_own_property(Object& this_object, const FlyString& property_nam
         dbg() << "Reconfigured property " << property_name << ", new shape says offset is " << metadata.value().offset << " and my storage capacity is " << m_storage.size();
     }
 
-    if (!new_property && mode == PutOwnPropertyMode::Put && !(metadata.value().attributes & Attribute::Writable)) {
+    auto value_here = m_storage[metadata.value().offset];
+    if (!new_property && mode == PutOwnPropertyMode::Put && !value_here.is_accessor() && !(metadata.value().attributes & Attribute::Writable)) {
         dbg() << "Disallow write to non-writable property";
         return false;
     }
@@ -254,7 +320,6 @@ bool Object::put_own_property(Object& this_object, const FlyString& property_nam
     if (value.is_empty())
         return true;
 
-    auto value_here = m_storage[metadata.value().offset];
     if (value_here.is_object() && value_here.as_object().is_native_property()) {
         auto& native_property = static_cast<NativeProperty&>(value_here.as_object());
         auto& interpreter = const_cast<Object*>(this)->interpreter();
@@ -377,6 +442,10 @@ bool Object::put(const FlyString& property_name, Value value, u8 attributes)
         auto metadata = object->shape().lookup(property_name);
         if (metadata.has_value()) {
             auto value_here = object->m_storage[metadata.value().offset];
+            if (value_here.is_accessor()) {
+                value_here.as_accessor().call_setter(Value(this), value);
+                return true;
+            }
             if (value_here.is_object() && value_here.as_object().is_native_property()) {
                 auto& native_property = static_cast<NativeProperty&>(value_here.as_object());
                 auto& interpreter = const_cast<Object*>(this)->interpreter();

Libraries/LibJS/Runtime/Shape.h

@@ -40,6 +40,8 @@ struct Attribute {
         Configurable = 1 << 0,
         Enumerable = 1 << 1,
         Writable = 1 << 2,
+        HasGet = 1 << 3,
+        HasSet = 1 << 4,
     };
 };
 

Libraries/LibJS/Runtime/Value.cpp

@@ -29,6 +29,7 @@
 #include <AK/StringBuilder.h>
 #include <LibJS/Heap/Heap.h>
 #include <LibJS/Interpreter.h>
+#include <LibJS/Runtime/Accessor.h>
 #include <LibJS/Runtime/Array.h>
 #include <LibJS/Runtime/BooleanObject.h>
 #include <LibJS/Runtime/Error.h>
@@ -63,6 +64,12 @@ Function& Value::as_function()
     return static_cast<Function&>(as_object());
 }
 
+Accessor& Value::as_accessor()
+{
+    ASSERT(is_accessor());
+    return static_cast<Accessor&>(*m_value.as_accessor);
+}
+
 String Value::to_string_without_side_effects() const
 {
     if (is_boolean())
@@ -92,6 +99,9 @@ String Value::to_string_without_side_effects() const
     if (is_symbol())
         return as_symbol().to_string();
 
+    if (is_accessor())
+        return "<accessor>";
+
     ASSERT(is_object());
     return String::format("[object %s]", as_object().class_name());
 }
@@ -205,6 +215,7 @@ Value Value::to_number(Interpreter& interpreter) const
 {
     switch (m_type) {
     case Type::Empty:
+    case Type::Accessor:
         ASSERT_NOT_REACHED();
         return {};
     case Type::Undefined:

Libraries/LibJS/Runtime/Value.h

@@ -45,6 +45,7 @@ public:
         Object,
         Boolean,
         Symbol,
+        Accessor,
     };
 
     bool is_empty() const { return m_type == Type::Empty; }
@@ -55,7 +56,8 @@ public:
     bool is_object() const { return m_type == Type::Object; }
     bool is_boolean() const { return m_type == Type::Boolean; }
     bool is_symbol() const { return m_type == Type::Symbol; }
-    bool is_cell() const { return is_string() || is_object(); }
+    bool is_accessor() const { return m_type == Type::Accessor; };
+    bool is_cell() const { return is_string() || is_accessor() || is_object(); }
     bool is_array() const;
     bool is_function() const;
 
@@ -119,6 +121,12 @@ public:
         m_value.as_symbol = symbol;
     }
 
+    Value(Accessor* accessor)
+        : m_type(Type::Accessor)
+    {
+        m_value.as_accessor = accessor;
+    }
+
     explicit Value(Type type)
         : m_type(type)
     {
@@ -183,6 +191,7 @@ public:
     String to_string_without_side_effects() const;
 
     Function& as_function();
+    Accessor& as_accessor();
 
     i32 as_i32() const;
     size_t as_size_t() const;
@@ -214,6 +223,7 @@ private:
         Symbol* as_symbol;
         Object* as_object;
         Cell* as_cell;
+        Accessor* as_accessor;
     } m_value;
 };
 

Libraries/LibJS/Tests/Object.defineProperty.js

@@ -40,6 +40,81 @@ try {
     assert(d.writable === true);
     assert(d.value === 9);
 
+    Object.defineProperty(o, "qux", {
+        configurable: true,
+        get() {
+            return o.secret_qux + 1;
+        },
+        set(value) {
+            this.secret_qux = value + 1;
+        },
+    });
+
+    o.qux = 10;
+    assert(o.qux === 12);
+    o.qux = 20;
+    assert(o.qux = 22);
+
+    Object.defineProperty(o, "qux", { configurable: true, value: 4 });
+
+    assert(o.qux === 4);
+    o.qux = 5;
+    assert(o.qux = 4);
+
+    Object.defineProperty(o, "qux", {
+        configurable: false,
+        get() {
+            return this.secret_qux + 2;
+        },
+        set(value) {
+            o.secret_qux = value + 2;
+        },
+    });
+
+    o.qux = 10;
+    assert(o.qux === 14);
+    o.qux = 20;
+    assert(o.qux = 24);
+
+    assertThrowsError(() => {
+        Object.defineProperty(o, "qux", {
+            configurable: false,
+            get() {
+                return this.secret_qux + 2;
+            },
+        });
+    }, {
+        error: TypeError,
+        message: "Cannot change attributes of non-configurable property 'qux'",
+    });
+
+    assertThrowsError(() => {
+        Object.defineProperty(o, "qux", { value: 2 });
+    }, {
+        error: TypeError,
+        message: "Cannot change attributes of non-configurable property 'qux'",
+    });
+
+    assertThrowsError(() => {
+        Object.defineProperty(o, "a", {
+            get() {},
+            value: 9,
+        });
+    }, {
+        error: TypeError,
+        message: "Accessor property descriptors cannot specify a value or writable key",
+    });
+
+    assertThrowsError(() => {
+        Object.defineProperty(o, "a", {
+            set() {},
+            writable: true,
+        });
+    }, {
+        error: TypeError,
+        message: "Accessor property descriptors cannot specify a value or writable key",
+    });
+
     console.log("PASS");
 } catch (e) {
     console.log(e)