LibJS: Implement [[Call]] and [[Construct]] internal slots properly

This patch implements:

- Spec compliant [[Call]] and [[Construct]] internal slots, as virtual
  FunctionObject::internal_{call,construct}(). These effectively replace
  the old virtual FunctionObject::{call,construct}(), but with several
  advantages:
  - Clear and consistent naming, following the object internal methods
  - Use of completions
  - internal_construct() returns an Object, and not Value! This has been
    a source of confusion for a long time, since in the spec there's
    always an Object returned but the Value return type in LibJS meant
    that this could not be fully trusted and something could screw you
    over.
  - Arguments are passed explicitly in form of a MarkedValueList,
    allowing manipulation (BoundFunction). We still put them on the
    execution context as a lot of code depends on it (VM::arguments()),
    but not from the Call() / Construct() AOs anymore, which now allows
    for bypassing them and invoking [[Call]] / [[Construct]] directly.
    Nothing but Call() / Construct() themselves do that at the moment,
    but future additions to ECMA262 or already existing web specs might.
- Spec compliant, standalone Call() and Construct() AOs: currently the
  closest we have is VM::{call,construct}(), but those try to cater to
  all the different function object subclasses at once, resulting in a
  horrible mess and calling AOs with functions they should never be
  called with; most prominently PrepareForOrdinaryCall and
  OrdinaryCallBindThis, which are only for ECMAScriptFunctionObject.

As a result this also contains an implicit optimization: we no longer
need to create a new function environment for NativeFunctions - which,
worth mentioning, is what started this whole crusade in the first place
:^)

Userland/Libraries/LibJS/AST.cpp

@@ -388,7 +388,7 @@ Value SuperCall::execute(Interpreter& interpreter, GlobalObject& global_object)
 
     // 11. Perform ? InitializeInstanceElements(result, F).
     VERIFY(result.is_object());
-    vm.initialize_instance_elements(result.as_object(), f);
+    TRY_OR_DISCARD(vm.initialize_instance_elements(result.as_object(), f));
 
     // 12. Return result.
     return result;
@@ -1205,6 +1205,8 @@ 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);

Userland/Libraries/LibJS/Runtime/AbstractOperations.cpp

@@ -42,6 +42,38 @@ ThrowCompletionOr<Value> require_object_coercible(GlobalObject& global_object, V
     return value;
 }
 
+// 7.3.13 Call ( F, V [ , argumentsList ] ), https://tc39.es/ecma262/#sec-call
+ThrowCompletionOr<Value> call_impl(GlobalObject& global_object, Value function, Value this_value, Optional<MarkedValueList> arguments_list)
+{
+    auto& vm = global_object.vm();
+
+    // 1. If argumentsList is not present, set argumentsList to a new empty List.
+    if (!arguments_list.has_value())
+        arguments_list = MarkedValueList { global_object.heap() };
+
+    // 2. If IsCallable(F) is false, throw a TypeError exception.
+    if (!function.is_function())
+        return vm.throw_completion<TypeError>(global_object, ErrorType::NotAFunction, function.to_string_without_side_effects());
+
+    // 3. Return ? F.[[Call]](V, argumentsList).
+    return function.as_function().internal_call(this_value, move(*arguments_list));
+}
+
+// 7.3.14 Construct ( F [ , argumentsList [ , newTarget ] ] ), https://tc39.es/ecma262/#sec-construct
+ThrowCompletionOr<Object*> construct(GlobalObject& global_object, FunctionObject& function, Optional<MarkedValueList> arguments_list, FunctionObject* new_target)
+{
+    // 1. If newTarget is not present, set newTarget to F.
+    if (!new_target)
+        new_target = &function;
+
+    // 2. If argumentsList is not present, set argumentsList to a new empty List.
+    if (!arguments_list.has_value())
+        arguments_list = MarkedValueList { global_object.heap() };
+
+    // 3. Return ? F.[[Construct]](argumentsList, newTarget).
+    return function.internal_construct(move(*arguments_list), *new_target);
+}
+
 // 7.3.18 LengthOfArrayLike ( obj ), https://tc39.es/ecma262/#sec-lengthofarraylike
 ThrowCompletionOr<size_t> length_of_array_like(GlobalObject& global_object, Object const& object)
 {

Userland/Libraries/LibJS/Runtime/AbstractOperations.h

@@ -20,6 +20,8 @@ Environment& get_this_environment(VM&);
 Object* get_super_constructor(VM&);
 ThrowCompletionOr<Reference> make_super_property_reference(GlobalObject&, Value actual_this, StringOrSymbol const& property_key, bool strict);
 ThrowCompletionOr<Value> require_object_coercible(GlobalObject&, Value);
+ThrowCompletionOr<Value> call_impl(GlobalObject&, Value function, Value this_value, Optional<MarkedValueList> = {});
+ThrowCompletionOr<Object*> construct(GlobalObject&, FunctionObject&, Optional<MarkedValueList> = {}, FunctionObject* new_target = nullptr);
 ThrowCompletionOr<size_t> length_of_array_like(GlobalObject&, Object const&);
 ThrowCompletionOr<MarkedValueList> create_list_from_array_like(GlobalObject&, Value, Function<ThrowCompletionOr<void>(Value)> = {});
 ThrowCompletionOr<FunctionObject*> species_constructor(GlobalObject&, Object const&, FunctionObject& default_constructor);
@@ -44,6 +46,25 @@ ThrowCompletionOr<Value> perform_eval(Value, GlobalObject&, CallerMode, EvalMode
 
 ThrowCompletionOr<void> eval_declaration_instantiation(VM& vm, GlobalObject& global_object, Program const& program, Environment* variable_environment, Environment* lexical_environment, bool strict);
 
+// 7.3.13 Call ( F, V [ , argumentsList ] ), https://tc39.es/ecma262/#sec-call
+template<typename... Args>
+ALWAYS_INLINE ThrowCompletionOr<Value> call(GlobalObject& global_object, Value function, Value this_value, MarkedValueList arguments_list)
+{
+    return call_impl(global_object, function, this_value, move(arguments_list));
+}
+
+template<typename... Args>
+ALWAYS_INLINE ThrowCompletionOr<Value> call(GlobalObject& global_object, Value function, Value this_value, Args... args)
+{
+    if constexpr (sizeof...(Args) > 0) {
+        MarkedValueList arguments_list { global_object.heap() };
+        (..., arguments_list.append(move(args)));
+        return call_impl(global_object, function, this_value, move(arguments_list));
+    }
+
+    return call_impl(global_object, function, this_value);
+}
+
 // 10.1.13 OrdinaryCreateFromConstructor ( constructor, intrinsicDefaultProto [ , internalSlotsList ] ), https://tc39.es/ecma262/#sec-ordinarycreatefromconstructor
 template<typename T, typename... Args>
 ThrowCompletionOr<T*> ordinary_create_from_constructor(GlobalObject& global_object, FunctionObject const& constructor, Object* (GlobalObject::*intrinsic_default_prototype)(), Args&&... args)

Userland/Libraries/LibJS/Runtime/BoundFunction.cpp

@@ -1,9 +1,11 @@
 /*
  * Copyright (c) 2020, Jack Karamanian <karamanian.jack@gmail.com>
+ * Copyright (c) 2021, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
 
+#include <LibJS/Runtime/AbstractOperations.h>
 #include <LibJS/Runtime/BoundFunction.h>
 #include <LibJS/Runtime/GlobalObject.h>
 
@@ -31,16 +33,51 @@ BoundFunction::~BoundFunction()
 {
 }
 
-Value BoundFunction::call()
+// 10.4.1.1 [[Call]] ( thisArgument, argumentsList ), https://tc39.es/ecma262/#sec-bound-function-exotic-objects-call-thisargument-argumentslist
+ThrowCompletionOr<Value> BoundFunction::internal_call([[maybe_unused]] Value this_argument, MarkedValueList arguments_list)
 {
-    return m_bound_target_function->call();
+    // 1. Let target be F.[[BoundTargetFunction]].
+    auto& target = *m_bound_target_function;
+
+    // 2. Let boundThis be F.[[BoundThis]].
+    auto bound_this = m_bound_this;
+
+    // 3. Let boundArgs be F.[[BoundArguments]].
+    auto& bound_args = m_bound_arguments;
+
+    // 4. Let args be the list-concatenation of boundArgs and argumentsList.
+    auto args = MarkedValueList { heap() };
+    args.extend(bound_args);
+    args.extend(move(arguments_list));
+
+    // 5. Return ? Call(target, boundThis, args).
+    return call(global_object(), &target, bound_this, move(args));
 }
 
-Value BoundFunction::construct(FunctionObject& new_target)
+// 10.4.1.2 [[Construct]] ( argumentsList, newTarget ), https://tc39.es/ecma262/#sec-bound-function-exotic-objects-construct-argumentslist-newtarget
+ThrowCompletionOr<Object*> BoundFunction::internal_construct(MarkedValueList arguments_list, FunctionObject& new_target)
 {
-    if (auto this_value = vm().this_value(global_object()); m_constructor_prototype && this_value.is_object())
-        TRY_OR_DISCARD(this_value.as_object().internal_set_prototype_of(m_constructor_prototype));
-    return m_bound_target_function->construct(new_target);
+    // 1. Let target be F.[[BoundTargetFunction]].
+    auto& target = *m_bound_target_function;
+
+    // 2. Assert: IsConstructor(target) is true.
+    VERIFY(Value(&target).is_constructor());
+
+    // 3. Let boundArgs be F.[[BoundArguments]].
+    auto& bound_args = m_bound_arguments;
+
+    // 4. Let args be the list-concatenation of boundArgs and argumentsList.
+    auto args = MarkedValueList { heap() };
+    args.extend(bound_args);
+    args.extend(move(arguments_list));
+
+    // 5. If SameValue(F, newTarget) is true, set newTarget to target.
+    auto* final_new_target = &new_target;
+    if (this == &new_target)
+        final_new_target = &target;
+
+    // 6. Return ? Construct(target, args, newTarget).
+    return construct(global_object(), target, move(args), final_new_target);
 }
 
 FunctionEnvironment* BoundFunction::new_function_environment(Object* new_target)

Userland/Libraries/LibJS/Runtime/BoundFunction.h

@@ -18,8 +18,9 @@ public:
     virtual void initialize(GlobalObject&) override;
     virtual ~BoundFunction();
 
-    virtual Value call() override;
-    virtual Value construct(FunctionObject& new_target) override;
+    virtual ThrowCompletionOr<Value> internal_call(Value this_argument, MarkedValueList arguments_list) override;
+    virtual ThrowCompletionOr<Object*> internal_construct(MarkedValueList arguments_list, FunctionObject& new_target) override;
+
     virtual FunctionEnvironment* new_function_environment(Object* new_target) override;
     virtual const FlyString& name() const override { return m_name; }
     virtual bool is_strict_mode() const override { return m_bound_target_function->is_strict_mode(); }

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp

@@ -97,6 +97,171 @@ ECMAScriptFunctionObject::~ECMAScriptFunctionObject()
 {
 }
 
+// 10.2.1 [[Call]] ( thisArgument, argumentsList ), https://tc39.es/ecma262/#sec-ecmascript-function-objects-call-thisargument-argumentslist
+ThrowCompletionOr<Value> ECMAScriptFunctionObject::internal_call(Value this_argument, MarkedValueList arguments_list)
+{
+    auto& vm = this->vm();
+
+    // 1. Let callerContext be the running execution context.
+    // NOTE: No-op, kept by the VM in its execution context stack.
+
+    ExecutionContext callee_context(heap());
+
+    // Non-standard
+    callee_context.arguments.extend(move(arguments_list));
+    if (auto* interpreter = vm.interpreter_if_exists())
+        callee_context.current_node = interpreter->current_node();
+
+    // 2. Let calleeContext be PrepareForOrdinaryCall(F, undefined).
+    vm.prepare_for_ordinary_call(*this, callee_context, nullptr);
+
+    // NOTE: We throw if the end of the native stack is reached, so unlike in the spec this _does_ need an exception check.
+    if (auto* exception = vm.exception())
+        return throw_completion(exception->value());
+
+    // 3. Assert: calleeContext is now the running execution context.
+    VERIFY(&vm.running_execution_context() == &callee_context);
+
+    // 4. If F.[[IsClassConstructor]] is true, then
+    if (m_is_class_constructor) {
+        // a. Let error be a newly created TypeError object.
+        // b. NOTE: error is created in calleeContext with F's associated Realm Record.
+        auto throw_completion = vm.throw_completion<TypeError>(global_object(), ErrorType::ClassConstructorWithoutNew, m_name);
+
+        // c. Remove calleeContext from the execution context stack and restore callerContext as the running execution context.
+        vm.pop_execution_context();
+
+        // d. Return ThrowCompletion(error).
+        return throw_completion;
+    }
+
+    // 5. Perform OrdinaryCallBindThis(F, calleeContext, thisArgument).
+    vm.ordinary_call_bind_this(*this, callee_context, this_argument);
+
+    // 6. Let result be OrdinaryCallEvaluateBody(F, argumentsList).
+    auto result = ordinary_call_evaluate_body();
+
+    // 7. Remove calleeContext from the execution context stack and restore callerContext as the running execution context.
+    vm.pop_execution_context();
+
+    // 8. If result.[[Type]] is return, return NormalCompletion(result.[[Value]]).
+    if (result.type() == Completion::Type::Return)
+        return result.value();
+
+    // 9. ReturnIfAbrupt(result).
+    if (result.is_abrupt()) {
+        // NOTE: I'm not sure if EvaluateBody can return a completion other than Normal, Return, or Throw.
+        // We're far from using completions in the AST anyway; in the meantime assume Throw.
+        VERIFY(result.is_error());
+        return result;
+    }
+
+    // 10. Return NormalCompletion(undefined).
+    return js_undefined();
+}
+
+// 10.2.2 [[Construct]] ( argumentsList, newTarget ), https://tc39.es/ecma262/#sec-ecmascript-function-objects-construct-argumentslist-newtarget
+ThrowCompletionOr<Object*> ECMAScriptFunctionObject::internal_construct(MarkedValueList arguments_list, FunctionObject& new_target)
+{
+    auto& vm = this->vm();
+    auto& global_object = this->global_object();
+
+    // 1. Let callerContext be the running execution context.
+    // NOTE: No-op, kept by the VM in its execution context stack.
+
+    // 2. Let kind be F.[[ConstructorKind]].
+    auto kind = m_constructor_kind;
+
+    Object* this_argument = nullptr;
+
+    // 3. If kind is base, then
+    if (kind == ConstructorKind::Base) {
+        // a. Let thisArgument be ? OrdinaryCreateFromConstructor(newTarget, "%Object.prototype%").
+        this_argument = TRY(ordinary_create_from_constructor<Object>(global_object, new_target, &GlobalObject::object_prototype));
+    }
+
+    ExecutionContext callee_context(heap());
+
+    // Non-standard
+    callee_context.arguments.extend(move(arguments_list));
+    if (auto* interpreter = vm.interpreter_if_exists())
+        callee_context.current_node = interpreter->current_node();
+
+    // 4. Let calleeContext be PrepareForOrdinaryCall(F, newTarget).
+    vm.prepare_for_ordinary_call(*this, callee_context, &new_target);
+
+    // NOTE: We throw if the end of the native stack is reached, so unlike in the spec this _does_ need an exception check.
+    if (auto* exception = vm.exception())
+        return throw_completion(exception->value());
+
+    // 5. Assert: calleeContext is now the running execution context.
+    VERIFY(&vm.running_execution_context() == &callee_context);
+
+    // 6. If kind is base, then
+    if (kind == ConstructorKind::Base) {
+        // a. Perform OrdinaryCallBindThis(F, calleeContext, thisArgument).
+        vm.ordinary_call_bind_this(*this, callee_context, this_argument);
+
+        // b. Let initializeResult be InitializeInstanceElements(thisArgument, F).
+        auto initialize_result = vm.initialize_instance_elements(*this_argument, *this);
+
+        // c. If initializeResult is an abrupt completion, then
+        if (initialize_result.is_throw_completion()) {
+            // i. Remove calleeContext from the execution context stack and restore callerContext as the running execution context.
+            vm.pop_execution_context();
+
+            // ii. Return Completion(initializeResult).
+            return initialize_result.throw_completion();
+        }
+    }
+
+    // 7. Let constructorEnv be the LexicalEnvironment of calleeContext.
+    auto* constructor_env = callee_context.lexical_environment;
+
+    // 8. Let result be OrdinaryCallEvaluateBody(F, argumentsList).
+    auto result = ordinary_call_evaluate_body();
+
+    // 9. Remove calleeContext from the execution context stack and restore callerContext as the running execution context.
+    vm.pop_execution_context();
+
+    // 10. If result.[[Type]] is return, then
+    if (result.type() == Completion::Type::Return) {
+        // FIXME: This is leftover from untangling the call/construct mess - doesn't belong here in any way, but removing it breaks derived classes.
+        // Likely fixed by making ClassDefinitionEvaluation fully spec compliant.
+        if (kind == ConstructorKind::Derived && result.value().is_object()) {
+            auto prototype = TRY(new_target.get(vm.names.prototype));
+            if (prototype.is_object())
+                TRY(result.value().as_object().internal_set_prototype_of(&prototype.as_object()));
+        }
+        // EOF (End of FIXME)
+
+        // a. If Type(result.[[Value]]) is Object, return NormalCompletion(result.[[Value]]).
+        if (result.value().is_object())
+            return &result.value().as_object();
+
+        // b. If kind is base, return NormalCompletion(thisArgument).
+        if (kind == ConstructorKind::Base)
+            return this_argument;
+
+        // c. If result.[[Value]] is not undefined, throw a TypeError exception.
+        if (!result.value().is_undefined())
+            return vm.throw_completion<TypeError>(global_object, ErrorType::DerivedConstructorReturningInvalidValue);
+    }
+    // 11. Else, ReturnIfAbrupt(result).
+    else {
+        // NOTE: I'm not sure if EvaluateBody can return a completion other than Normal, Return, or Throw.
+        // We're far from using completions in the AST anyway; in the meantime assume Throw.
+        VERIFY(result.is_error());
+        return result;
+    }
+
+    // 12. Return ? constructorEnv.GetThisBinding().
+    auto this_binding = constructor_env->get_this_binding(global_object);
+    if (auto* exception = vm.exception())
+        return throw_completion(exception->value());
+    return &this_binding.as_object();
+}
+
 void ECMAScriptFunctionObject::visit_edges(Visitor& visitor)
 {
     Base::visit_edges(visitor);
@@ -392,14 +557,15 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::function_declaration_instantia
     return {};
 }
 
-Value ECMAScriptFunctionObject::execute_function_body()
+// 10.2.1.4 OrdinaryCallEvaluateBody ( F, argumentsList ), https://tc39.es/ecma262/#sec-ordinarycallevaluatebody
+Completion ECMAScriptFunctionObject::ordinary_call_evaluate_body()
 {
     auto& vm = this->vm();
     auto* bytecode_interpreter = Bytecode::Interpreter::current();
 
     if (bytecode_interpreter) {
         // FIXME: pass something to evaluate default arguments with
-        TRY_OR_DISCARD(function_declaration_instantiation(nullptr));
+        TRY(function_declaration_instantiation(nullptr));
         if (!m_bytecode_executable.has_value()) {
             m_bytecode_executable = Bytecode::Generator::generate(m_ecmascript_code, m_kind == FunctionKind::Generator);
             auto& passes = JS::Bytecode::Interpreter::optimization_pipeline();
@@ -412,10 +578,14 @@ Value ECMAScriptFunctionObject::execute_function_body()
             }
         }
         auto result = bytecode_interpreter->run(*m_bytecode_executable);
+        if (auto* exception = vm.exception())
+            return throw_completion(exception->value());
+        // NOTE: Running the bytecode should eventually return a completion.
+        // Until it does, we assume "return" and include the undefined fallback from the call site.
         if (m_kind != FunctionKind::Generator)
-            return result;
+            return { Completion::Type::Return, result.value_or(js_undefined()), {} };
 
-        return GeneratorObject::create(global_object(), result, this, vm.running_execution_context().lexical_environment, bytecode_interpreter->snapshot_frame());
+        return normal_completion(GeneratorObject::create(global_object(), result, this, vm.running_execution_context().lexical_environment, bytecode_interpreter->snapshot_frame()));
     } else {
         VERIFY(m_kind != FunctionKind::Generator);
         OwnPtr<Interpreter> local_interpreter;
@@ -428,28 +598,16 @@ Value ECMAScriptFunctionObject::execute_function_body()
 
         VM::InterpreterExecutionScope scope(*ast_interpreter);
 
-        TRY_OR_DISCARD(function_declaration_instantiation(ast_interpreter));
+        TRY(function_declaration_instantiation(ast_interpreter));
 
-        return m_ecmascript_code->execute(*ast_interpreter, global_object());
+        auto result = m_ecmascript_code->execute(*ast_interpreter, global_object());
+        if (auto* exception = vm.exception())
+            return throw_completion(exception->value());
+        // NOTE: Running the AST node should eventually return a completion.
+        // Until it does, we assume "return" and include the undefined fallback from the call site.
+        return { Completion::Type::Return, result.value_or(js_undefined()), {} };
     }
-}
-
-Value ECMAScriptFunctionObject::call()
-{
-    if (m_is_class_constructor) {
-        vm().throw_exception<TypeError>(global_object(), ErrorType::ClassConstructorWithoutNew, m_name);
-        return {};
-    }
-    return execute_function_body();
-}
-
-Value ECMAScriptFunctionObject::construct(FunctionObject&)
-{
-    if (m_is_arrow_function || m_kind == FunctionKind::Generator) {
-        vm().throw_exception<TypeError>(global_object(), ErrorType::NotAConstructor, m_name);
-        return {};
-    }
-    return execute_function_body();
+    VERIFY_NOT_REACHED();
 }
 
 void ECMAScriptFunctionObject::set_name(const FlyString& name)

Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h

@@ -34,12 +34,12 @@ public:
     virtual void initialize(GlobalObject&) override;
     virtual ~ECMAScriptFunctionObject();
 
+    virtual ThrowCompletionOr<Value> internal_call(Value this_argument, MarkedValueList arguments_list) override;
+    virtual ThrowCompletionOr<Object*> internal_construct(MarkedValueList arguments_list, FunctionObject& new_target) override;
+
     Statement const& ecmascript_code() const { return m_ecmascript_code; }
     Vector<FunctionNode::Parameter> const& formal_parameters() const { return m_formal_parameters; };
 
-    virtual Value call() override;
-    virtual Value construct(FunctionObject& new_target) override;
-
     virtual const FlyString& name() const override { return m_name; };
     void set_name(const FlyString& name);
 
@@ -71,7 +71,8 @@ public:
     // This is for IsSimpleParameterList (static semantics)
     bool has_simple_parameter_list() const { return m_has_simple_parameter_list; }
 
-    virtual bool has_constructor() const override { return true; }
+    // Equivalent to absence of [[Construct]]
+    virtual bool has_constructor() const override { return !(m_is_arrow_function || m_kind == FunctionKind::Generator); }
 
 protected:
     virtual bool is_strict_mode() const final { return m_strict; }
@@ -82,7 +83,7 @@ private:
     virtual void visit_edges(Visitor&) override;
 
     ThrowCompletionOr<void> function_declaration_instantiation(Interpreter*);
-    Value execute_function_body();
+    Completion ordinary_call_evaluate_body();
 
     // Internal Slots of ECMAScript Function Objects, https://tc39.es/ecma262/#table-internal-slots-of-ecmascript-function-objects
     Environment* m_environment { nullptr };                       // [[Environment]]

Userland/Libraries/LibJS/Runtime/FunctionObject.h

@@ -18,8 +18,11 @@ public:
     virtual ~FunctionObject();
     virtual void initialize(GlobalObject&) override { }
 
-    virtual Value call() = 0;
-    virtual Value construct(FunctionObject& new_target) = 0;
+    // Table 7: Additional Essential Internal Methods of Function Objects, https://tc39.es/ecma262/#table-additional-essential-internal-methods-of-function-objects
+
+    virtual ThrowCompletionOr<Value> internal_call(Value this_argument, MarkedValueList arguments_list) = 0;
+    virtual ThrowCompletionOr<Object*> internal_construct([[maybe_unused]] MarkedValueList arguments_list, [[maybe_unused]] FunctionObject& new_target) { VERIFY_NOT_REACHED(); }
+
     virtual const FlyString& name() const = 0;
     virtual FunctionEnvironment* new_function_environment(Object* new_target) = 0;
 

Userland/Libraries/LibJS/Runtime/NativeFunction.cpp

@@ -1,5 +1,6 @@
 /*
  * Copyright (c) 2020, Andreas Kling <kling@serenityos.org>
+ * Copyright (c) 2021, Linus Groh <linusg@serenityos.org>
  *
  * SPDX-License-Identifier: BSD-2-Clause
  */
@@ -46,6 +47,143 @@ NativeFunction::~NativeFunction()
 {
 }
 
+// NOTE: Do not attempt to DRY these, it's not worth it. The difference in return types (Value vs Object*),
+// called functions (call() vs construct(FunctionObject&)), and this value (passed vs uninitialized) make
+// these good candidates for a bit of code duplication :^)
+
+// 10.3.1 [[Call]] ( thisArgument, argumentsList ), https://tc39.es/ecma262/#sec-built-in-function-objects-call-thisargument-argumentslist
+ThrowCompletionOr<Value> NativeFunction::internal_call(Value this_argument, MarkedValueList arguments_list)
+{
+    auto& vm = this->vm();
+    auto& global_object = this->global_object();
+
+    // 1. Let callerContext be the running execution context.
+    auto& caller_context = vm.running_execution_context();
+
+    // 2. If callerContext is not already suspended, suspend callerContext.
+    // NOTE: We don't support this concept yet.
+
+    // 3. Let calleeContext be a new execution context.
+    ExecutionContext callee_context(heap());
+
+    // 4. Set the Function of calleeContext to F.
+    callee_context.function = this;
+    callee_context.function_name = m_name;
+
+    // 5. Let calleeRealm be F.[[Realm]].
+    auto* callee_realm = m_realm;
+    // NOTE: This non-standard fallback is needed until we can guarantee that literally
+    // every function has a realm - especially in LibWeb that's sometimes not the case
+    // when a function is created while no JS is running, as we currently need to rely on
+    // that (:acid2:, I know - see set_event_handler_attribute() for an example).
+    // If there's no 'current realm' either, we can't continue and crash.
+    if (!callee_realm)
+        callee_realm = vm.current_realm();
+    VERIFY(callee_realm);
+
+    // 6. Set the Realm of calleeContext to calleeRealm.
+    callee_context.realm = callee_realm;
+
+    // 7. Set the ScriptOrModule of calleeContext to null.
+    // FIXME: Our execution context struct currently does not track this item.
+
+    // 8. Perform any necessary implementation-defined initialization of calleeContext.
+
+    callee_context.this_value = this_argument;
+    callee_context.arguments.extend(move(arguments_list));
+
+    callee_context.lexical_environment = caller_context.lexical_environment;
+    callee_context.variable_environment = caller_context.variable_environment;
+
+    // NOTE: This is a LibJS specific hack for NativeFunction to inherit the strictness of its caller.
+    callee_context.is_strict_mode = vm.in_strict_mode();
+
+    if (auto* interpreter = vm.interpreter_if_exists())
+        callee_context.current_node = interpreter->current_node();
+
+    // </8.> --------------------------------------------------------------------------
+
+    // 9. Push calleeContext onto the execution context stack; calleeContext is now the running execution context.
+    vm.push_execution_context(callee_context, global_object);
+
+    // 10. Let result be the Completion Record that is the result of evaluating F in a manner that conforms to the specification of F. thisArgument is the this value, argumentsList provides the named parameters, and the NewTarget value is undefined.
+    auto result = call();
+
+    // 11. Remove calleeContext from the execution context stack and restore callerContext as the running execution context.
+    vm.pop_execution_context();
+
+    // 12. Return result.
+    if (auto* exception = vm.exception())
+        return throw_completion(exception->value());
+    return result;
+}
+
+// 10.3.2 [[Construct]] ( argumentsList, newTarget ), https://tc39.es/ecma262/#sec-built-in-function-objects-construct-argumentslist-newtarget
+ThrowCompletionOr<Object*> NativeFunction::internal_construct(MarkedValueList arguments_list, FunctionObject& new_target)
+{
+    auto& vm = this->vm();
+    auto& global_object = this->global_object();
+
+    // 1. Let callerContext be the running execution context.
+    auto& caller_context = vm.running_execution_context();
+
+    // 2. If callerContext is not already suspended, suspend callerContext.
+    // NOTE: We don't support this concept yet.
+
+    // 3. Let calleeContext be a new execution context.
+    ExecutionContext callee_context(heap());
+
+    // 4. Set the Function of calleeContext to F.
+    callee_context.function = this;
+    callee_context.function_name = m_name;
+
+    // 5. Let calleeRealm be F.[[Realm]].
+    auto* callee_realm = m_realm;
+    // NOTE: This non-standard fallback is needed until we can guarantee that literally
+    // every function has a realm - especially in LibWeb that's sometimes not the case
+    // when a function is created while no JS is running, as we currently need to rely on
+    // that (:acid2:, I know - see set_event_handler_attribute() for an example).
+    // If there's no 'current realm' either, we can't continue and crash.
+    if (!callee_realm)
+        callee_realm = vm.current_realm();
+    VERIFY(callee_realm);
+
+    // 6. Set the Realm of calleeContext to calleeRealm.
+    callee_context.realm = callee_realm;
+
+    // 7. Set the ScriptOrModule of calleeContext to null.
+    // FIXME: Our execution context struct currently does not track this item.
+
+    // 8. Perform any necessary implementation-defined initialization of calleeContext.
+
+    callee_context.arguments.extend(move(arguments_list));
+
+    callee_context.lexical_environment = caller_context.lexical_environment;
+    callee_context.variable_environment = caller_context.variable_environment;
+
+    // NOTE: This is a LibJS specific hack for NativeFunction to inherit the strictness of its caller.
+    callee_context.is_strict_mode = vm.in_strict_mode();
+
+    if (auto* interpreter = vm.interpreter_if_exists())
+        callee_context.current_node = interpreter->current_node();
+
+    // </8.> --------------------------------------------------------------------------
+
+    // 9. Push calleeContext onto the execution context stack; calleeContext is now the running execution context.
+    vm.push_execution_context(callee_context, global_object);
+
+    // 10. Let result be the Completion Record that is the result of evaluating F in a manner that conforms to the specification of F. The this value is uninitialized, argumentsList provides the named parameters, and newTarget provides the NewTarget value.
+    auto result = construct(new_target);
+
+    // 11. Remove calleeContext from the execution context stack and restore callerContext as the running execution context.
+    vm.pop_execution_context();
+
+    // 12. Return result.
+    if (auto* exception = vm.exception())
+        return throw_completion(exception->value());
+    return &result.as_object();
+}
+
 Value NativeFunction::call()
 {
     return m_native_function(vm(), global_object());
@@ -53,7 +191,8 @@ Value NativeFunction::call()
 
 Value NativeFunction::construct(FunctionObject&)
 {
-    return {};
+    // Needs to be overridden if [[Construct]] is needed.
+    VERIFY_NOT_REACHED();
 }
 
 FunctionEnvironment* NativeFunction::new_function_environment(Object* new_target)

Userland/Libraries/LibJS/Runtime/NativeFunction.h

@@ -21,8 +21,14 @@ public:
     virtual void initialize(GlobalObject&) override { }
     virtual ~NativeFunction() override;
 
-    virtual Value call() override;
-    virtual Value construct(FunctionObject& new_target) override;
+    virtual ThrowCompletionOr<Value> internal_call(Value this_argument, MarkedValueList arguments_list) override;
+    virtual ThrowCompletionOr<Object*> internal_construct(MarkedValueList arguments_list, FunctionObject& new_target) override;
+
+    // Used for [[Call]] / [[Construct]]'s "...result of evaluating F in a manner that conforms to the specification of F".
+    // Needs to be overridden by all NativeFunctions without an m_native_function.
+    virtual Value call();
+    virtual Value construct(FunctionObject& new_target);
+
     virtual const FlyString& name() const override { return m_name; };
     virtual bool is_strict_mode() const override;
     virtual bool has_constructor() const override { return false; }

Userland/Libraries/LibJS/Runtime/ProxyObject.cpp

@@ -762,98 +762,86 @@ ThrowCompletionOr<MarkedValueList> ProxyObject::internal_own_property_keys() con
 }
 
 // 10.5.12 [[Call]] ( thisArgument, argumentsList ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-call-thisargument-argumentslist
-Value ProxyObject::call()
+ThrowCompletionOr<Value> ProxyObject::internal_call(Value this_argument, MarkedValueList arguments_list)
 {
     auto& vm = this->vm();
     auto& global_object = this->global_object();
-    auto this_argument = vm.this_value(global_object);
 
     // A Proxy exotic object only has a [[Call]] internal method if the initial value of its [[ProxyTarget]] internal slot is an object that has a [[Call]] internal method.
-    if (!is_function()) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::NotAFunction, Value(this).to_string_without_side_effects());
-        return {};
-    }
+    // TODO: We should be able to turn this into a VERIFY(), this must be checked at the call site.
+    //       According to the spec, the Call() AO may be called with a non-function argument, but
+    //       throws before calling [[Call]]() if that's the case.
+    if (!is_function())
+        return vm.throw_completion<TypeError>(global_object, ErrorType::NotAFunction, Value(this).to_string_without_side_effects());
 
     // 1. Let handler be O.[[ProxyHandler]].
 
     // 2. If handler is null, throw a TypeError exception.
-    if (m_is_revoked) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyRevoked);
-        return {};
-    }
+    if (m_is_revoked)
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyRevoked);
 
     // 3. Assert: Type(handler) is Object.
     // 4. Let target be O.[[ProxyTarget]].
 
     // 5. Let trap be ? GetMethod(handler, "apply").
-    auto trap = TRY_OR_DISCARD(Value(&m_handler).get_method(global_object, vm.names.apply));
+    auto trap = TRY(Value(&m_handler).get_method(global_object, vm.names.apply));
 
     // 6. If trap is undefined, then
     if (!trap) {
         // a. Return ? Call(target, thisArgument, argumentsList).
-        return static_cast<FunctionObject&>(m_target).call();
+        return call(global_object, &m_target, this_argument, move(arguments_list));
     }
 
     // 7. Let argArray be ! CreateArrayFromList(argumentsList).
-    auto arguments_array = Array::create(global_object, 0);
-    vm.for_each_argument([&](auto& argument) {
-        arguments_array->indexed_properties().append(argument);
-    });
+    auto* arguments_array = Array::create_from(global_object, arguments_list);
 
     // 8. Return ? Call(trap, handler, « target, thisArgument, argArray »).
-    return TRY_OR_DISCARD(vm.call(*trap, &m_handler, &m_target, this_argument, arguments_array));
+    return call(global_object, trap, &m_handler, &m_target, this_argument, arguments_array);
 }
 
 // 10.5.13 [[Construct]] ( argumentsList, newTarget ), https://tc39.es/ecma262/#sec-proxy-object-internal-methods-and-internal-slots-construct-argumentslist-newtarget
-Value ProxyObject::construct(FunctionObject& new_target)
+ThrowCompletionOr<Object*> ProxyObject::internal_construct(MarkedValueList arguments_list, FunctionObject& new_target)
 {
     auto& vm = this->vm();
     auto& global_object = this->global_object();
 
     // A Proxy exotic object only has a [[Construct]] internal method if the initial value of its [[ProxyTarget]] internal slot is an object that has a [[Construct]] internal method.
-    if (!is_function()) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::NotAConstructor, Value(this).to_string_without_side_effects());
-        return {};
-    }
+    // TODO: We should be able to turn this into a VERIFY(), this must be checked at the call site.
+    //       According to the spec, the Construct() AO is only ever called with a constructor argument.
+    if (!is_function())
+        return vm.throw_completion<TypeError>(global_object, ErrorType::NotAConstructor, Value(this).to_string_without_side_effects());
 
     // 1. Let handler be O.[[ProxyHandler]].
 
     // 2. If handler is null, throw a TypeError exception.
-    if (m_is_revoked) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyRevoked);
-        return {};
-    }
+    if (m_is_revoked)
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyRevoked);
 
     // 3. Assert: Type(handler) is Object.
     // 4. Let target be O.[[ProxyTarget]].
     // 5. Assert: IsConstructor(target) is true.
 
     // 6. Let trap be ? GetMethod(handler, "construct").
-    auto trap = TRY_OR_DISCARD(Value(&m_handler).get_method(global_object, vm.names.construct));
+    auto trap = TRY(Value(&m_handler).get_method(global_object, vm.names.construct));
 
     // 7. If trap is undefined, then
     if (!trap) {
         // a. Return ? Construct(target, argumentsList, newTarget).
-        return static_cast<FunctionObject&>(m_target).construct(new_target);
+        return construct(global_object, static_cast<FunctionObject&>(m_target), move(arguments_list), &new_target);
     }
 
     // 8. Let argArray be ! CreateArrayFromList(argumentsList).
-    auto arguments_array = Array::create(global_object, 0);
-    vm.for_each_argument([&](auto& argument) {
-        arguments_array->indexed_properties().append(argument);
-    });
+    auto* arguments_array = Array::create_from(global_object, arguments_list);
 
     // 9. Let newObj be ? Call(trap, handler, « target, argArray, newTarget »).
-    auto result = TRY_OR_DISCARD(vm.call(*trap, &m_handler, &m_target, arguments_array, &new_target));
+    auto new_object = TRY(call(global_object, trap, &m_handler, &m_target, arguments_array, &new_target));
 
     // 10. If Type(newObj) is not Object, throw a TypeError exception.
-    if (!result.is_object()) {
-        vm.throw_exception<TypeError>(global_object, ErrorType::ProxyConstructBadReturnType);
-        return {};
-    }
+    if (!new_object.is_object())
+        return vm.throw_completion<TypeError>(global_object, ErrorType::ProxyConstructBadReturnType);
 
     // 11. Return newObj.
-    return result;
+    return &new_object.as_object();
 }
 
 void ProxyObject::visit_edges(Cell::Visitor& visitor)

Userland/Libraries/LibJS/Runtime/ProxyObject.h

@@ -21,8 +21,6 @@ public:
     ProxyObject(Object& target, Object& handler, Object& prototype);
     virtual ~ProxyObject() override;
 
-    virtual Value call() override;
-    virtual Value construct(FunctionObject& new_target) override;
     virtual const FlyString& name() const override;
     virtual FunctionEnvironment* new_function_environment(Object* new_target) override;
     virtual bool has_constructor() const override { return true; }
@@ -46,6 +44,8 @@ public:
     virtual ThrowCompletionOr<bool> internal_set(PropertyName const&, Value value, Value receiver) override;
     virtual ThrowCompletionOr<bool> internal_delete(PropertyName const&) override;
     virtual ThrowCompletionOr<MarkedValueList> internal_own_property_keys() const override;
+    virtual ThrowCompletionOr<Value> internal_call(Value this_argument, MarkedValueList arguments_list) override;
+    virtual ThrowCompletionOr<Object*> internal_construct(MarkedValueList arguments_list, FunctionObject& new_target) override;
 
 private:
     virtual void visit_edges(Visitor&) override;

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -13,6 +13,7 @@
 #include <LibJS/Runtime/AbstractOperations.h>
 #include <LibJS/Runtime/Array.h>
 #include <LibJS/Runtime/BoundFunction.h>
+#include <LibJS/Runtime/Completion.h>
 #include <LibJS/Runtime/ECMAScriptFunctionObject.h>
 #include <LibJS/Runtime/Error.h>
 #include <LibJS/Runtime/FinalizationRegistry.h>
@@ -477,116 +478,21 @@ Reference VM::resolve_binding(FlyString const& name, Environment* environment)
     return get_identifier_reference(environment, name, strict);
 }
 
-static void append_bound_and_passed_arguments(MarkedValueList& arguments, Vector<Value> bound_arguments, Optional<MarkedValueList> passed_arguments)
-{
-    arguments.ensure_capacity(bound_arguments.size());
-    arguments.extend(move(bound_arguments));
-
-    if (passed_arguments.has_value()) {
-        auto arguments_list = move(passed_arguments.release_value().values());
-        arguments.grow_capacity(arguments_list.size());
-        arguments.extend(move(arguments_list));
-    }
-}
-
 // 7.3.32 InitializeInstanceElements ( O, constructor ), https://tc39.es/ecma262/#sec-initializeinstanceelements
-void VM::initialize_instance_elements(Object& object, ECMAScriptFunctionObject& constructor)
+ThrowCompletionOr<void> VM::initialize_instance_elements(Object& object, ECMAScriptFunctionObject& constructor)
 {
     for (auto& field : constructor.fields()) {
         field.define_field(*this, object);
-        if (exception())
-            return;
+        if (auto* exception = this->exception())
+            return JS::throw_completion(exception->value());
     }
+    return {};
 }
 
-// FIXME: This function should not exist as-is, most of it should be moved to the individual
-//        [[Construct]] implementations so that this becomes the Construct() AO (3 steps).
+// NOTE: This is a leftover from the old world of vm.call() and vm.construct(). Replace all uses with plain construct() and remove this.
 Value VM::construct(FunctionObject& function, FunctionObject& new_target, Optional<MarkedValueList> arguments)
 {
-    auto& global_object = function.global_object();
-
-    Value this_argument;
-    if (!is<ECMAScriptFunctionObject>(function) || static_cast<ECMAScriptFunctionObject&>(function).constructor_kind() == ECMAScriptFunctionObject::ConstructorKind::Base)
-        this_argument = TRY_OR_DISCARD(ordinary_create_from_constructor<Object>(global_object, new_target, &GlobalObject::object_prototype));
-
-    // FIXME: prepare_for_ordinary_call() is not supposed to receive a BoundFunction, ProxyObject, etc. - ever.
-    //        This needs to be moved to NativeFunction/ECMAScriptFunctionObject's construct() (10.2.2 [[Construct]])
-    ExecutionContext callee_context(heap());
-    prepare_for_ordinary_call(function, callee_context, &new_target);
-    if (exception())
-        return {};
-
-    ArmedScopeGuard pop_guard = [&] {
-        pop_execution_context();
-    };
-
-    if (auto* interpreter = interpreter_if_exists())
-        callee_context.current_node = interpreter->current_node();
-
-    if (is<BoundFunction>(function)) {
-        auto& bound_function = static_cast<BoundFunction&>(function);
-        append_bound_and_passed_arguments(callee_context.arguments, bound_function.bound_arguments(), move(arguments));
-    } else {
-        append_bound_and_passed_arguments(callee_context.arguments, {}, move(arguments));
-    }
-
-    if (auto* environment = callee_context.lexical_environment) {
-        auto& function_environment = verify_cast<FunctionEnvironment>(*environment);
-        function_environment.set_new_target(&new_target);
-        if (!this_argument.is_empty() && function_environment.this_binding_status() != FunctionEnvironment::ThisBindingStatus::Lexical) {
-            function_environment.bind_this_value(global_object, this_argument);
-            if (exception())
-                return {};
-        }
-    }
-
-    // If we are a Derived constructor, |this| has not been constructed before super is called.
-    callee_context.this_value = this_argument;
-
-    if (is<ECMAScriptFunctionObject>(function) && static_cast<ECMAScriptFunctionObject&>(function).constructor_kind() == ECMAScriptFunctionObject::ConstructorKind::Base) {
-        VERIFY(this_argument.is_object());
-        initialize_instance_elements(this_argument.as_object(), static_cast<ECMAScriptFunctionObject&>(function));
-        if (exception())
-            return {};
-    }
-
-    auto* constructor_environment = callee_context.lexical_environment;
-    auto result = function.construct(new_target);
-    VERIFY(constructor_environment);
-
-    pop_execution_context();
-    pop_guard.disarm();
-
-    // If we are constructing an instance of a derived class,
-    // set the prototype on objects created by constructors that return an object (i.e. NativeFunction subclasses).
-
-    if ((!is<ECMAScriptFunctionObject>(function) || static_cast<ECMAScriptFunctionObject&>(function).constructor_kind() == ECMAScriptFunctionObject::ConstructorKind::Base)
-        && is<ECMAScriptFunctionObject>(new_target) && static_cast<ECMAScriptFunctionObject&>(new_target).constructor_kind() == ECMAScriptFunctionObject::ConstructorKind::Derived
-        && result.is_object()) {
-        verify_cast<FunctionEnvironment>(constructor_environment)->replace_this_binding(result);
-
-        auto prototype = TRY_OR_DISCARD(new_target.get(names.prototype));
-        if (prototype.is_object())
-            TRY_OR_DISCARD(result.as_object().internal_set_prototype_of(&prototype.as_object()));
-        return result;
-    }
-
-    if (exception())
-        return {};
-
-    if (result.is_object())
-        return result;
-
-    if (is<ECMAScriptFunctionObject>(function) && static_cast<ECMAScriptFunctionObject&>(function).constructor_kind() == ECMAScriptFunctionObject::ConstructorKind::Base)
-        return this_argument;
-
-    if (!result.is_empty() && !result.is_undefined()) {
-        throw_exception<TypeError>(global_object, ErrorType::DerivedConstructorReturningInvalidValue);
-        return {};
-    }
-
-    VERIFY(constructor_environment);
-    return constructor_environment->get_this_binding(global_object);
+    return TRY_OR_DISCARD(JS::construct(function.global_object(), function, move(arguments), &new_target));
 }
 
 void VM::throw_exception(Exception& exception)
@@ -622,12 +528,8 @@ Value VM::get_new_target()
 // 10.2.1.1 PrepareForOrdinaryCall ( F, newTarget ), https://tc39.es/ecma262/#sec-prepareforordinarycall
 void VM::prepare_for_ordinary_call(FunctionObject& function, ExecutionContext& callee_context, Object* new_target)
 {
-    // NOTE: This is a LibJS specific hack for NativeFunction to inherit the strictness of its caller.
-    // FIXME: I feel like we should be able to get rid of this.
-    if (is<NativeFunction>(function))
-        callee_context.is_strict_mode = in_strict_mode();
-    else
-        callee_context.is_strict_mode = function.is_strict_mode();
+    // Non-standard
+    callee_context.is_strict_mode = function.is_strict_mode();
 
     // 1. Let callerContext be the running execution context.
     // 2. Let calleeContext be a new ECMAScript code execution context.
@@ -642,9 +544,13 @@ void VM::prepare_for_ordinary_call(FunctionObject& function, ExecutionContext& c
 
     // 4. Let calleeRealm be F.[[Realm]].
     auto* callee_realm = function.realm();
-    // FIXME: See FIXME in VM::call_internal() / VM::construct().
+    // NOTE: This non-standard fallback is needed until we can guarantee that literally
+    // every function has a realm - especially in LibWeb that's sometimes not the case
+    // when a function is created while no JS is running, as we currently need to rely on
+    // that (:acid2:, I know - see set_event_handler_attribute() for an example).
+    // If there's no 'current realm' either, we can't continue and crash.
     if (!callee_realm)
-        callee_realm = current_realm();
+        callee_realm = vm.current_realm();
     VERIFY(callee_realm);
 
     // 5. Set the Realm of calleeContext to calleeRealm.
@@ -703,46 +609,14 @@ void VM::ordinary_call_bind_this(FunctionObject& function, ExecutionContext& cal
     callee_context.this_value = this_value;
 }
 
+// NOTE: This is only here because there's a million invocations of vm.call() - it used to be tied to the VM in weird ways.
+// We should update all of those and then remove this, along with the call() template functions in VM.h, and use the standalone call() AO.
 ThrowCompletionOr<Value> VM::call_internal(FunctionObject& function, Value this_value, Optional<MarkedValueList> arguments)
 {
     VERIFY(!exception());
     VERIFY(!this_value.is_empty());
 
-    if (is<BoundFunction>(function)) {
-        auto& bound_function = static_cast<BoundFunction&>(function);
-
-        MarkedValueList with_bound_arguments { heap() };
-        append_bound_and_passed_arguments(with_bound_arguments, bound_function.bound_arguments(), move(arguments));
-
-        return call_internal(bound_function.bound_target_function(), bound_function.bound_this(), move(with_bound_arguments));
-    }
-
-    // FIXME: prepare_for_ordinary_call() is not supposed to receive a BoundFunction, ProxyObject, etc. - ever.
-    //        This needs to be moved to NativeFunction/ECMAScriptFunctionObject's construct() (10.2.2 [[Construct]])
-    ExecutionContext callee_context(heap());
-    prepare_for_ordinary_call(function, callee_context, nullptr);
-    if (auto* exception = this->exception())
-        return JS::throw_completion(exception->value());
-
-    ScopeGuard pop_guard = [&] {
-        pop_execution_context();
-    };
-
-    if (auto* interpreter = interpreter_if_exists())
-        callee_context.current_node = interpreter->current_node();
-
-    callee_context.this_value = this_value;
-    append_bound_and_passed_arguments(callee_context.arguments, {}, move(arguments));
-
-    ordinary_call_bind_this(function, callee_context, this_value);
-
-    if (auto* exception = this->exception())
-        return JS::throw_completion(exception->value());
-
-    auto result = function.call();
-    if (auto* exception = this->exception())
-        return JS::throw_completion(exception->value());
-    return result;
+    return JS::call_impl(function.global_object(), &function, this_value, move(arguments));
 }
 
 bool VM::in_strict_mode() const

Userland/Libraries/LibJS/Runtime/VM.h

@@ -133,15 +133,6 @@ public:
 
     bool in_strict_mode() const;
 
-    template<typename Callback>
-    void for_each_argument(Callback callback)
-    {
-        if (m_execution_context_stack.is_empty())
-            return;
-        for (auto& value : running_execution_context().arguments)
-            callback(value);
-    }
-
     size_t argument_count() const
     {
         if (m_execution_context_stack.is_empty())
@@ -248,9 +239,9 @@ public:
     [[nodiscard]] ALWAYS_INLINE ThrowCompletionOr<Value> call(FunctionObject& function, Value this_value, Args... args)
     {
         if constexpr (sizeof...(Args) > 0) {
-            MarkedValueList arglist { heap() };
-            (..., arglist.append(move(args)));
-            return call(function, this_value, move(arglist));
+            MarkedValueList arguments_list { heap() };
+            (..., arguments_list.append(move(args)));
+            return call(function, this_value, move(arguments_list));
         }
 
         return call(function, this_value);
@@ -270,7 +261,7 @@ public:
     Function<void(const Promise&)> on_promise_unhandled_rejection;
     Function<void(const Promise&)> on_promise_rejection_handled;
 
-    void initialize_instance_elements(Object& object, ECMAScriptFunctionObject& constructor);
+    ThrowCompletionOr<void> initialize_instance_elements(Object& object, ECMAScriptFunctionObject& constructor);
 
     CustomData* custom_data() { return m_custom_data; }
 
@@ -283,13 +274,14 @@ public:
     void save_execution_context_stack();
     void restore_execution_context_stack();
 
+    // TODO: Move these elsewhere once only used for ECMAScriptFunctionObject.
+    void prepare_for_ordinary_call(FunctionObject&, ExecutionContext& callee_context, Object* new_target);
+    void ordinary_call_bind_this(FunctionObject&, ExecutionContext&, Value this_argument);
+
 private:
     explicit VM(OwnPtr<CustomData>);
 
-    void ordinary_call_bind_this(FunctionObject&, ExecutionContext&, Value this_argument);
-
     [[nodiscard]] ThrowCompletionOr<Value> call_internal(FunctionObject&, Value this_value, Optional<MarkedValueList> arguments);
-    void prepare_for_ordinary_call(FunctionObject&, ExecutionContext& callee_context, Object* new_target);
 
     ThrowCompletionOr<Object*> copy_data_properties(Object& rest_object, Object const& source, HashTable<PropertyName, PropertyNameTraits> const& seen_names, GlobalObject& global_object);
 

Userland/Libraries/LibJS/Tests/builtins/Reflect/Reflect.construct.js

@@ -14,7 +14,7 @@ describe("errors", () => {
     test("arguments list must be an object", () => {
         [null, undefined, "foo", 123, NaN, Infinity].forEach(value => {
             expect(() => {
-                Reflect.construct(() => {}, value);
+                Reflect.construct(function () {}, value);
             }).toThrowWithMessage(TypeError, `${value} is not an object`);
         });
     });
@@ -22,7 +22,7 @@ describe("errors", () => {
     test("new target must be a constructor", () => {
         [null, undefined, "foo", 123, NaN, Infinity, {}].forEach(value => {
             expect(() => {
-                Reflect.construct(() => {}, [], value);
+                Reflect.construct(function () {}, [], value);
             }).toThrowWithMessage(TypeError, `${value} is not a constructor`);
         });
     });

Userland/Libraries/LibJS/Tests/functions/arrow-functions.js

@@ -146,7 +146,10 @@ test("cannot be constructed", () => {
     let foo = () => {};
     expect(() => {
         new foo();
-    }).toThrowWithMessage(TypeError, "foo is not a constructor");
+    }).toThrowWithMessage(
+        TypeError,
+        "[object ECMAScriptFunctionObject] is not a constructor (evaluated from 'foo')"
+    );
 });
 
 test("syntax errors", () => {