LibJS: Use new NativeFunction::create() in most places

Resolves one FIXME where we can now pass a realm, and sets the length
correctly in a bunch of places that previously didn't.
Also reduces the number of "format function name string from arbitrary
PropertyKey" implementations, although two more remain present in the
AST (used with ECMAScriptFunctionObjects, which is a different beast).

Userland/Libraries/LibJS/CyclicModule.cpp

@@ -457,7 +457,7 @@ ThrowCompletionOr<void> CyclicModule::execute_async_module(VM& vm)
     };
 
     // 5. Let onFulfilled be ! CreateBuiltinFunction(fulfilledClosure, 0, "", « »).
-    auto* on_fulfilled = NativeFunction::create(global_object, "", move(fulfilled_closure));
+    auto* on_fulfilled = NativeFunction::create(global_object, move(fulfilled_closure), 0, "");
 
     // 6. Let rejectedClosure be a new Abstract Closure with parameters (error) that captures module and performs the following steps when called:
     auto rejected_closure = [&](VM& vm, GlobalObject&) -> ThrowCompletionOr<Value> {
@@ -470,8 +470,8 @@ ThrowCompletionOr<void> CyclicModule::execute_async_module(VM& vm)
         return js_undefined();
     };
 
-    auto* on_rejected = NativeFunction::create(global_object, "", move(rejected_closure));
     // 7. Let onRejected be ! CreateBuiltinFunction(rejectedClosure, 0, "", « »).
+    auto* on_rejected = NativeFunction::create(global_object, move(rejected_closure), 0, "");
 
     VERIFY(is<Promise>(*capability.promise));
 

Userland/Libraries/LibJS/Runtime/AsyncFromSyncIteratorPrototype.cpp

@@ -52,7 +52,7 @@ static ThrowCompletionOr<Object*> async_from_sync_iterator_continuation(GlobalOb
     };
 
     // 8. Let onFulfilled be ! CreateBuiltinFunction(unwrap, 1, "", « »).
-    auto on_fulfilled = NativeFunction::create(global_object, "", move(unwrap));
+    auto* on_fulfilled = NativeFunction::create(global_object, move(unwrap), 1, "");
     // 9. NOTE: onFulfilled is used when processing the "value" property of an IteratorResult object in order to wait for its value if it is a promise and re-package the result in a new "unwrapped" IteratorResult object.
     VERIFY(is<Promise>(value_wrapper));
     auto* value_wrapper_promise = static_cast<Promise*>(value_wrapper);

Userland/Libraries/LibJS/Runtime/Completion.cpp

@@ -63,7 +63,7 @@ ThrowCompletionOr<Value> await(GlobalObject& global_object, Value value)
     };
 
     // 4. Let onFulfilled be ! CreateBuiltinFunction(fulfilledClosure, 1, "", « »).
-    auto on_fulfilled = NativeFunction::create(global_object, "", move(fulfilled_closure));
+    auto* on_fulfilled = NativeFunction::create(global_object, move(fulfilled_closure), 1, "");
 
     // 5. Let rejectedClosure be a new Abstract Closure with parameters (reason) that captures asyncContext and performs the following steps when called:
     auto rejected_closure = [&success, &result](VM& vm, GlobalObject&) -> ThrowCompletionOr<Value> {
@@ -87,7 +87,7 @@ ThrowCompletionOr<Value> await(GlobalObject& global_object, Value value)
     };
 
     // 6. Let onRejected be ! CreateBuiltinFunction(rejectedClosure, 1, "", « »).
-    auto on_rejected = NativeFunction::create(global_object, "", move(rejected_closure));
+    auto* on_rejected = NativeFunction::create(global_object, move(rejected_closure), 1, "");
 
     // 7. Perform ! PerformPromiseThen(promise, onFulfilled, onRejected).
     auto* promise = verify_cast<Promise>(promise_object);

Userland/Libraries/LibJS/Runtime/Object.cpp

@@ -1052,29 +1052,12 @@ void Object::set_prototype(Object* new_prototype)
 
 void Object::define_native_accessor(PropertyKey const& property_key, Function<ThrowCompletionOr<Value>(VM&, GlobalObject&)> getter, Function<ThrowCompletionOr<Value>(VM&, GlobalObject&)> setter, PropertyAttributes attribute)
 {
-    auto& vm = this->vm();
-    String formatted_property_key;
-    if (property_key.is_number()) {
-        formatted_property_key = property_key.to_string();
-    } else if (property_key.is_string()) {
-        formatted_property_key = property_key.as_string();
-    } else {
-        formatted_property_key = String::formatted("[{}]", property_key.as_symbol()->description());
-    }
     FunctionObject* getter_function = nullptr;
-    if (getter) {
-        auto name = String::formatted("get {}", formatted_property_key);
-        getter_function = NativeFunction::create(global_object(), name, move(getter));
-        getter_function->define_direct_property(vm.names.length, Value(0), Attribute::Configurable);
-        getter_function->define_direct_property(vm.names.name, js_string(vm, name), Attribute::Configurable);
-    }
+    if (getter)
+        getter_function = NativeFunction::create(global_object(), move(getter), 0, property_key, {}, {}, "get"sv);
     FunctionObject* setter_function = nullptr;
-    if (setter) {
-        auto name = String::formatted("set {}", formatted_property_key);
-        setter_function = NativeFunction::create(global_object(), name, move(setter));
-        setter_function->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-        setter_function->define_direct_property(vm.names.name, js_string(vm, name), Attribute::Configurable);
-    }
+    if (setter)
+        setter_function = NativeFunction::create(global_object(), move(setter), 1, property_key, {}, {}, "set"sv);
     return define_direct_accessor(property_key, getter_function, setter_function, attribute);
 }
 
@@ -1118,16 +1101,7 @@ Value Object::get_without_side_effects(const PropertyKey& property_key) const
 
 void Object::define_native_function(PropertyKey const& property_key, Function<ThrowCompletionOr<Value>(VM&, GlobalObject&)> native_function, i32 length, PropertyAttributes attribute)
 {
-    auto& vm = this->vm();
-    String function_name;
-    if (property_key.is_string()) {
-        function_name = property_key.as_string();
-    } else {
-        function_name = String::formatted("[{}]", property_key.as_symbol()->description());
-    }
-    auto* function = NativeFunction::create(global_object(), function_name, move(native_function));
-    function->define_direct_property(vm.names.length, Value(length), Attribute::Configurable);
-    function->define_direct_property(vm.names.name, js_string(vm, function_name), Attribute::Configurable);
+    auto* function = NativeFunction::create(global_object(), move(native_function), length, property_key);
     define_direct_property(property_key, function, attribute);
 }
 

Userland/Libraries/LibJS/Runtime/PromisePrototype.cpp

@@ -99,8 +99,7 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
     // 6. Else,
     else {
         // a. Let thenFinallyClosure be a new Abstract Closure with parameters (value) that captures onFinally and C and performs the following steps when called:
-        // b. Let thenFinally be ! CreateBuiltinFunction(thenFinallyClosure, 1, "", « »).
-        auto* then_finally_function = NativeFunction::create(global_object, "", [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+        auto then_finally_closure = [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
             auto& constructor = const_cast<FunctionObject&>(*constructor_handle.cell());
             auto& on_finally = const_cast<FunctionObject&>(*on_finally_handle.cell());
             auto value = vm.argument(0);
@@ -112,21 +111,23 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
             auto* promise = TRY(promise_resolve(global_object, constructor, result));
 
             // iii. Let returnValue be a new Abstract Closure with no parameters that captures value and performs the following steps when called:
-            // iv. Let valueThunk be ! CreateBuiltinFunction(returnValue, 0, "", « »).
-            auto* value_thunk = NativeFunction::create(global_object, "", [value](auto&, auto&) -> ThrowCompletionOr<Value> {
+            auto return_value = [value](auto&, auto&) -> ThrowCompletionOr<Value> {
                 // 1. Return value.
                 return value;
-            });
+            };
+
+            // iv. Let valueThunk be ! CreateBuiltinFunction(returnValue, 0, "", « »).
+            auto* value_thunk = NativeFunction::create(global_object, move(return_value), 0, "");
 
             // v. Return ? Invoke(promise, "then", « valueThunk »).
             return TRY(Value(promise).invoke(global_object, vm.names.then, value_thunk));
-        });
-        then_finally_function->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-        then_finally = Value(then_finally_function);
+        };
+
+        // b. Let thenFinally be ! CreateBuiltinFunction(thenFinallyClosure, 1, "", « »).
+        then_finally = NativeFunction::create(global_object, move(then_finally_closure), 1, "");
 
         // c. Let catchFinallyClosure be a new Abstract Closure with parameters (reason) that captures onFinally and C and performs the following steps when called:
-        // d. Let catchFinally be ! CreateBuiltinFunction(catchFinallyClosure, 1, "", « »).
-        auto* catch_finally_function = NativeFunction::create(global_object, "", [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+        auto catch_finally_closure = [constructor_handle = make_handle(constructor), on_finally_handle = make_handle(&on_finally.as_function())](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
             auto& constructor = const_cast<FunctionObject&>(*constructor_handle.cell());
             auto& on_finally = const_cast<FunctionObject&>(*on_finally_handle.cell());
             auto reason = vm.argument(0);
@@ -137,17 +138,21 @@ JS_DEFINE_NATIVE_FUNCTION(PromisePrototype::finally)
             // ii. Let promise be ? PromiseResolve(C, result).
             auto* promise = TRY(promise_resolve(global_object, constructor, result));
 
-            // iv. Let thrower be ! CreateBuiltinFunction(throwReason, 0, "", « »).
-            auto* thrower = NativeFunction::create(global_object, "", [reason](auto&, auto&) -> ThrowCompletionOr<Value> {
+            // iii. Let throwReason be a new Abstract Closure with no parameters that captures reason and performs the following steps when called:
+            auto throw_reason = [reason](auto&, auto&) -> ThrowCompletionOr<Value> {
                 // 1. Return ThrowCompletion(reason).
                 return throw_completion(reason);
-            });
+            };
+
+            // iv. Let thrower be ! CreateBuiltinFunction(throwReason, 0, "", « »).
+            auto* thrower = NativeFunction::create(global_object, move(throw_reason), 0, "");
 
             // v. Return ? Invoke(promise, "then", « thrower »).
             return TRY(Value(promise).invoke(global_object, vm.names.then, thrower));
-        });
-        catch_finally_function->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-        catch_finally = Value(catch_finally_function);
+        };
+
+        // d. Let catchFinally be ! CreateBuiltinFunction(catchFinallyClosure, 1, "", « »).
+        catch_finally = NativeFunction::create(global_object, move(catch_finally_closure), 1, "");
     }
 
     // 7. Return ? Invoke(promise, "then", « thenFinally, catchFinally »).

Userland/Libraries/LibJS/Runtime/PromiseReaction.cpp

@@ -31,8 +31,7 @@ ThrowCompletionOr<PromiseCapability> new_promise_capability(GlobalObject& global
     } promise_capability_functions;
 
     // 4. Let executorClosure be a new Abstract Closure with parameters (resolve, reject) that captures promiseCapability and performs the following steps when called:
-    // 5. Let executor be ! CreateBuiltinFunction(executorClosure, 2, "", « »).
-    auto* executor = NativeFunction::create(global_object, "", [&promise_capability_functions](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+    auto executor_closure = [&promise_capability_functions](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
         auto resolve = vm.argument(0);
         auto reject = vm.argument(1);
 
@@ -53,9 +52,10 @@ ThrowCompletionOr<PromiseCapability> new_promise_capability(GlobalObject& global
 
         // e. Return undefined.
         return js_undefined();
-    });
-    executor->define_direct_property(vm.names.length, Value(2), Attribute::Configurable);
-    executor->define_direct_property(vm.names.name, js_string(vm, String::empty()), Attribute::Configurable);
+    };
+
+    // 5. Let executor be ! CreateBuiltinFunction(executorClosure, 2, "", « »).
+    auto* executor = NativeFunction::create(global_object, move(executor_closure), 2, "");
 
     // 6. Let promise be ? Construct(C, « executor »).
     auto* promise = TRY(construct(global_object, constructor.as_function(), executor));

Userland/Libraries/LibJS/Runtime/ProxyConstructor.cpp

@@ -60,25 +60,44 @@ ThrowCompletionOr<Object*> ProxyConstructor::construct(FunctionObject&)
 // 28.2.2.1 Proxy.revocable ( target, handler ), https://tc39.es/ecma262/#sec-proxy.revocable
 JS_DEFINE_NATIVE_FUNCTION(ProxyConstructor::revocable)
 {
+    // 1. Let p be ? ProxyCreate(target, handler).
     auto* proxy = TRY(proxy_create(global_object, vm.argument(0), vm.argument(1)));
 
-    // 28.2.2.1.1 Proxy Revocation Functions, https://tc39.es/ecma262/#sec-proxy-revocation-functions
-    auto* revoker = NativeFunction::create(global_object, "", [proxy_handle = make_handle(proxy)](auto&, auto&) -> ThrowCompletionOr<Value> {
+    // 2. Let revokerClosure be a new Abstract Closure with no parameters that captures nothing and performs the following steps when called:
+    auto revoker_closure = [proxy_handle = make_handle(proxy)](auto&, auto&) -> ThrowCompletionOr<Value> {
+        // a. Let F be the active function object.
+
+        // b. Let p be F.[[RevocableProxy]].
         auto& proxy = const_cast<ProxyObject&>(*proxy_handle.cell());
+
+        // c. If p is null, return undefined.
         if (proxy.is_revoked())
             return js_undefined();
-        // NOTE: The spec wants us to unset [[ProxyTarget]] and [[ProxyHandler]],
-        // which is their way of revoking the Proxy - this might affect GC-ability,
-        // but AFAICT not doing that should be ok compatibility-wise.
+
+        // d. Set F.[[RevocableProxy]] to null.
+        // e. Assert: p is a Proxy object.
+        // f. Set p.[[ProxyTarget]] to null.
+        // g. Set p.[[ProxyHandler]] to null.
         proxy.revoke();
+
+        // h. Return undefined.
         return js_undefined();
-    });
-    revoker->define_direct_property(vm.names.length, Value(0), Attribute::Configurable);
-    revoker->define_direct_property(vm.names.name, js_string(vm, String::empty()), Attribute::Configurable);
+    };
 
+    // 3. Let revoker be ! CreateBuiltinFunction(revokerClosure, 0, "", « [[RevocableProxy]] »).
+    // 4. Set revoker.[[RevocableProxy]] to p.
+    auto* revoker = NativeFunction::create(global_object, move(revoker_closure), 0, "");
+
+    // 5. Let result be ! OrdinaryObjectCreate(%Object.prototype%).
     auto* result = Object::create(global_object, global_object.object_prototype());
+
+    // 6. Perform ! CreateDataPropertyOrThrow(result, "proxy", p).
     MUST(result->create_data_property_or_throw(vm.names.proxy, proxy));
+
+    // 7. Perform ! CreateDataPropertyOrThrow(result, "revoke", revoker).
     MUST(result->create_data_property_or_throw(vm.names.revoke, revoker));
+
+    // 8. Return result.
     return result;
 }
 

Userland/Libraries/LibJS/Runtime/ShadowRealm.cpp

@@ -228,44 +228,39 @@ ThrowCompletionOr<Value> shadow_realm_import_value(GlobalObject& global_object,
     // NOTE: We don't support this concept yet.
 
     // 9. Let steps be the steps of an ExportGetter function as described below.
+    auto steps = [string = move(export_name_string)](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
+        // 1. Assert: exports is a module namespace exotic object.
+        VERIFY(vm.argument(0).is_object());
+        auto& exports = vm.argument(0).as_object();
+        VERIFY(is<ModuleNamespaceObject>(exports));
+
+        // 2. Let f be the active function object.
+        auto* function = vm.running_execution_context().function;
+
+        // 3. Let string be f.[[ExportNameString]].
+        // 4. Assert: Type(string) is String.
+
+        // 5. Let hasOwn be ? HasOwnProperty(exports, string).
+        auto has_own = TRY(exports.has_own_property(string));
+
+        // 6. If hasOwn is false, throw a TypeError exception.
+        if (!has_own)
+            return vm.template throw_completion<TypeError>(global_object, ErrorType::MissingRequiredProperty, string);
+
+        // 7. Let value be ? Get(exports, string).
+        auto value = TRY(exports.get(string));
+
+        // 8. Let realm be f.[[Realm]].
+        auto* realm = function->realm();
+        VERIFY(realm);
+
+        // 9. Return ? GetWrappedValue(realm, value).
+        return get_wrapped_value(global_object, *realm, value);
+    };
+
     // 10. Let onFulfilled be ! CreateBuiltinFunction(steps, 1, "", « [[ExportNameString]] », callerRealm).
     // 11. Set onFulfilled.[[ExportNameString]] to exportNameString.
-    // FIXME: Support passing a realm to NativeFunction::create()
-    (void)caller_realm;
-    auto* on_fulfilled = NativeFunction::create(
-        global_object,
-        "",
-        [string = move(export_name_string)](auto& vm, auto& global_object) -> ThrowCompletionOr<Value> {
-            // 1. Assert: exports is a module namespace exotic object.
-            VERIFY(vm.argument(0).is_object());
-            auto& exports = vm.argument(0).as_object();
-            VERIFY(is<ModuleNamespaceObject>(exports));
-
-            // 2. Let f be the active function object.
-            auto* function = vm.running_execution_context().function;
-
-            // 3. Let string be f.[[ExportNameString]].
-            // 4. Assert: Type(string) is String.
-
-            // 5. Let hasOwn be ? HasOwnProperty(exports, string).
-            auto has_own = TRY(exports.has_own_property(string));
-
-            // 6. If hasOwn is false, throw a TypeError exception.
-            if (!has_own)
-                return vm.template throw_completion<TypeError>(global_object, ErrorType::MissingRequiredProperty, string);
-
-            // 7. Let value be ? Get(exports, string).
-            auto value = TRY(exports.get(string));
-
-            // 8. Let realm be f.[[Realm]].
-            auto* realm = function->realm();
-            VERIFY(realm);
-
-            // 9. Return ? GetWrappedValue(realm, value).
-            return get_wrapped_value(global_object, *realm, value);
-        });
-    on_fulfilled->define_direct_property(vm.names.length, Value(1), Attribute::Configurable);
-    on_fulfilled->define_direct_property(vm.names.name, js_string(vm, String::empty()), Attribute::Configurable);
+    auto* on_fulfilled = NativeFunction::create(global_object, move(steps), 1, "", &caller_realm);
 
     // 12. Let promiseCapability be ! NewPromiseCapability(%Promise%).
     auto promise_capability = MUST(new_promise_capability(global_object, global_object.promise_constructor()));

Userland/Libraries/LibJS/Runtime/VM.cpp

@@ -997,7 +997,7 @@ void VM::finish_dynamic_import(ScriptOrModule referencing_script_or_module, Modu
     };
 
     // 2. Let onFulfilled be ! CreateBuiltinFunction(fulfilledClosure, 0, "", « »).
-    auto* on_fulfilled = NativeFunction::create(current_realm()->global_object(), "", move(fulfilled_closure));
+    auto* on_fulfilled = NativeFunction::create(current_realm()->global_object(), move(fulfilled_closure), 0, "");
 
     // 3. Let rejectedClosure be a new Abstract Closure with parameters (error) that captures promiseCapability and performs the following steps when called:
     auto rejected_closure = [rejected_function = make_handle(promise_capability.reject)](VM& vm, GlobalObject& global_object) -> ThrowCompletionOr<Value> {
@@ -1009,7 +1009,7 @@ void VM::finish_dynamic_import(ScriptOrModule referencing_script_or_module, Modu
     };
 
     // 4. Let onRejected be ! CreateBuiltinFunction(rejectedClosure, 0, "", « »).
-    auto* on_rejected = NativeFunction::create(current_realm()->global_object(), "", move(rejected_closure));
+    auto* on_rejected = NativeFunction::create(current_realm()->global_object(), move(rejected_closure), 0, "");
 
     // 5. Perform ! PerformPromiseThen(innerPromise, onFulfilled, onRejected).
     inner_promise->perform_then(on_fulfilled, on_rejected, {});