From 845da3901d40da3a9841a4d71a4c77fc8a27e5d4 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Mon, 27 Nov 2023 13:38:19 +0100 Subject: [PATCH] LibJS: Make ExecutionContext::function_name a GCPtr This required setting things up so that all function objects can plop a PrimitiveString there instead of an AK string. This is a step towards making ExecutionContext easier to allocate. --- Userland/Libraries/LibJS/Console.cpp | 4 ++-- .../LibJS/Runtime/ECMAScriptFunctionObject.cpp | 9 ++++++--- .../LibJS/Runtime/ECMAScriptFunctionObject.h | 2 ++ Userland/Libraries/LibJS/Runtime/Error.cpp | 2 +- .../LibJS/Runtime/ExecutionContext.cpp | 1 + .../Libraries/LibJS/Runtime/ExecutionContext.h | 2 +- .../Libraries/LibJS/Runtime/NativeFunction.cpp | 18 ++++++++++++++++-- .../Libraries/LibJS/Runtime/NativeFunction.h | 8 +++----- Userland/Libraries/LibJS/Runtime/VM.cpp | 4 ++-- 9 files changed, 34 insertions(+), 16 deletions(-) diff --git a/Userland/Libraries/LibJS/Console.cpp b/Userland/Libraries/LibJS/Console.cpp index 7e780dd636..0e202b8894 100644 --- a/Userland/Libraries/LibJS/Console.cpp +++ b/Userland/Libraries/LibJS/Console.cpp @@ -142,9 +142,9 @@ ThrowCompletionOr Console::trace() // NOTE: -2 to skip the console.trace() execution context for (ssize_t i = execution_context_stack.size() - 2; i >= 0; --i) { auto const& function_name = execution_context_stack[i]->function_name; - trace.stack.append(function_name.is_empty() + trace.stack.append((!function_name || function_name->is_empty()) ? ""_string - : TRY_OR_THROW_OOM(vm, String::from_deprecated_string(function_name))); + : function_name->utf8_string()); } // 2. Optionally, let formattedData be the result of Formatter(data), and incorporate formattedData as a label for trace. diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp index a87077153d..be43cac236 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp @@ -333,8 +333,10 @@ void ECMAScriptFunctionObject::initialize(Realm& realm) // which must give the properties in chronological order which in this case is the order they // are defined in the spec. + m_name_string = PrimitiveString::create(vm, m_name); + MUST(define_property_or_throw(vm.names.length, { .value = Value(m_function_length), .writable = false, .enumerable = false, .configurable = true })); - MUST(define_property_or_throw(vm.names.name, { .value = PrimitiveString::create(vm, m_name.is_null() ? "" : m_name), .writable = false, .enumerable = false, .configurable = true })); + MUST(define_property_or_throw(vm.names.name, { .value = m_name_string, .writable = false, .enumerable = false, .configurable = true })); if (!m_is_arrow_function) { Object* prototype = nullptr; @@ -936,7 +938,7 @@ ThrowCompletionOr ECMAScriptFunctionObject::prepare_for_ordinary_call(Exec // 3. Set the Function of calleeContext to F. callee_context.function = this; - callee_context.function_name = m_name; + callee_context.function_name = m_name_string; // 4. Let calleeRealm be F.[[Realm]]. auto callee_realm = m_realm; @@ -1243,6 +1245,7 @@ void ECMAScriptFunctionObject::set_name(DeprecatedFlyString const& name) VERIFY(!name.is_null()); auto& vm = this->vm(); m_name = name; - MUST(define_property_or_throw(vm.names.name, { .value = PrimitiveString::create(vm, m_name), .writable = false, .enumerable = false, .configurable = true })); + m_name_string = PrimitiveString::create(vm, m_name); + MUST(define_property_or_throw(vm.names.name, { .value = m_name_string, .writable = false, .enumerable = false, .configurable = true })); } } diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h index 401b3f970d..aa8704e71d 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.h @@ -112,6 +112,8 @@ private: ThrowCompletionOr function_declaration_instantiation(); DeprecatedFlyString m_name; + GCPtr m_name_string; + GCPtr m_bytecode_executable; Vector> m_default_parameter_bytecode_executables; i32 m_function_length { 0 }; diff --git a/Userland/Libraries/LibJS/Runtime/Error.cpp b/Userland/Libraries/LibJS/Runtime/Error.cpp index e01cc47d2b..11087619f9 100644 --- a/Userland/Libraries/LibJS/Runtime/Error.cpp +++ b/Userland/Libraries/LibJS/Runtime/Error.cpp @@ -85,7 +85,7 @@ void Error::populate_stack() if (element.source_range.has_value()) range = element.source_range.value(); TracebackFrame frame { - .function_name = context->function_name, + .function_name = context->function_name ? context->function_name->deprecated_string() : "", .source_range_storage = range, }; diff --git a/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp b/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp index 1964cf5cf5..ea0255c23b 100644 --- a/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp +++ b/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp @@ -53,6 +53,7 @@ void ExecutionContext::visit_edges(Cell::Visitor& visitor) visitor.visit(this_value); if (instruction_stream_iterator.has_value()) visitor.visit(const_cast(instruction_stream_iterator.value().executable())); + visitor.visit(function_name); script_or_module.visit( [](Empty) {}, [&](auto& script_or_module) { diff --git a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h index f9f0d9197a..970c9fb305 100644 --- a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h +++ b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h @@ -49,7 +49,7 @@ public: GCPtr context_owner; Optional instruction_stream_iterator; - DeprecatedFlyString function_name; + GCPtr function_name; Value this_value; MarkedVector arguments; MarkedVector local_variables; diff --git a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp index c55655ff87..ea82639edb 100644 --- a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp +++ b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp @@ -17,6 +17,20 @@ namespace JS { JS_DEFINE_ALLOCATOR(NativeFunction); +void NativeFunction::initialize(Realm& realm) +{ + Base::initialize(realm); + m_name_string = PrimitiveString::create(vm(), m_name); +} + +void NativeFunction::visit_edges(Cell::Visitor& visitor) +{ + Base::visit_edges(visitor); + visitor.visit(m_native_function); + visitor.visit(m_realm); + visitor.visit(m_name_string); +} + // 10.3.3 CreateBuiltinFunction ( behaviour, length, name, additionalInternalSlotsList [ , realm [ , prototype [ , prefix ] ] ] ), https://tc39.es/ecma262/#sec-createbuiltinfunction // NOTE: This doesn't consider additionalInternalSlotsList, which is rarely used, and can either be implemented using only the `function` lambda, or needs a NativeFunction subclass. NonnullGCPtr NativeFunction::create(Realm& allocating_realm, Function(VM&)> behaviour, i32 length, PropertyKey const& name, Optional realm, Optional prototype, Optional const& prefix) @@ -111,7 +125,7 @@ ThrowCompletionOr NativeFunction::internal_call(Value this_argument, Read // 4. Set the Function of calleeContext to F. callee_context.function = this; - callee_context.function_name = m_name; + callee_context.function_name = m_name_string; // 5. Let calleeRealm be F.[[Realm]]. auto callee_realm = m_realm; @@ -175,7 +189,7 @@ ThrowCompletionOr> NativeFunction::internal_construct(Reado // 4. Set the Function of calleeContext to F. callee_context.function = this; - callee_context.function_name = m_name; + callee_context.function_name = m_name_string; // 5. Let calleeRealm be F.[[Realm]]. auto callee_realm = m_realm; diff --git a/Userland/Libraries/LibJS/Runtime/NativeFunction.h b/Userland/Libraries/LibJS/Runtime/NativeFunction.h index 67287ae0be..338dc272dc 100644 --- a/Userland/Libraries/LibJS/Runtime/NativeFunction.h +++ b/Userland/Libraries/LibJS/Runtime/NativeFunction.h @@ -48,16 +48,14 @@ protected: NativeFunction(DeprecatedFlyString name, JS::GCPtr(VM&)>>, Object& prototype); explicit NativeFunction(Object& prototype); - virtual void visit_edges(Cell::Visitor& visitor) override - { - Base::visit_edges(visitor); - visitor.visit(m_native_function); - } + virtual void initialize(Realm&) override; + virtual void visit_edges(Cell::Visitor& visitor) override; private: virtual bool is_native_function() const final { return true; } DeprecatedFlyString m_name; + GCPtr m_name_string; Optional m_initial_name; // [[InitialName]] JS::GCPtr(VM&)>> m_native_function; GCPtr m_realm; diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index f7401dc397..b0fddba974 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -749,9 +749,9 @@ void VM::dump_backtrace() const auto& frame = m_execution_context_stack[i]; if (frame->instruction_stream_iterator.has_value() && frame->instruction_stream_iterator->source_code()) { auto source_range = frame->instruction_stream_iterator->source_range().realize(); - dbgln("-> {} @ {}:{},{}", frame->function_name, source_range.filename(), source_range.start.line, source_range.start.column); + dbgln("-> {} @ {}:{},{}", frame->function_name ? frame->function_name->utf8_string() : ""_string, source_range.filename(), source_range.start.line, source_range.start.column); } else { - dbgln("-> {}", frame->function_name); + dbgln("-> {}", frame->function_name ? frame->function_name->utf8_string() : ""_string); } } }