From 50bf303edd573e765dda8416376b9e83e4a86966 Mon Sep 17 00:00:00 2001 From: Hediadyoin1 Date: Thu, 27 Jul 2023 14:40:01 +0200 Subject: [PATCH] LibJS: Capture UnrealizedSourceRanges in ExecutionContext, not ASTNodes This loosens the connection to the AST interpreter and will allow us to generate SourceRanges for the Bytecode interpreter in the future as well Moves UnrealizedSourceRanges from TracebackFrame to the JS namespace for this --- Userland/Applications/Spreadsheet/Workbook.cpp | 1 - Userland/Libraries/LibJS/AST.cpp | 2 +- Userland/Libraries/LibJS/AST.h | 4 ++++ .../LibJS/Runtime/ECMAScriptFunctionObject.cpp | 8 ++++---- Userland/Libraries/LibJS/Runtime/Error.cpp | 15 ++------------- Userland/Libraries/LibJS/Runtime/Error.h | 5 ----- .../Libraries/LibJS/Runtime/ExecutionContext.cpp | 2 +- .../Libraries/LibJS/Runtime/ExecutionContext.h | 3 ++- .../Libraries/LibJS/Runtime/NativeFunction.cpp | 9 +++++---- Userland/Libraries/LibJS/Runtime/VM.cpp | 4 ++-- Userland/Libraries/LibJS/SourceRange.h | 12 ++++++++++++ 11 files changed, 33 insertions(+), 32 deletions(-) diff --git a/Userland/Applications/Spreadsheet/Workbook.cpp b/Userland/Applications/Spreadsheet/Workbook.cpp index 1bf24dfbe6..2a61a1764f 100644 --- a/Userland/Applications/Spreadsheet/Workbook.cpp +++ b/Userland/Applications/Spreadsheet/Workbook.cpp @@ -29,7 +29,6 @@ Workbook::Workbook(Vector>&& sheets, GUI::Window& parent_wi m_workbook_object = m_vm->heap().allocate(m_interpreter->realm(), m_interpreter->realm(), *this).release_allocated_value_but_fixme_should_propagate_errors(); m_interpreter->realm().global_object().define_direct_property("workbook", workbook_object(), JS::default_attributes); - m_main_execution_context.current_node = nullptr; m_main_execution_context.this_value = &m_interpreter->realm().global_object(); m_main_execution_context.function_name = "(global execution context)"sv; m_main_execution_context.lexical_environment = &m_interpreter->realm().global_environment(); diff --git a/Userland/Libraries/LibJS/AST.cpp b/Userland/Libraries/LibJS/AST.cpp index 5bf9938c3d..45080a44fd 100644 --- a/Userland/Libraries/LibJS/AST.cpp +++ b/Userland/Libraries/LibJS/AST.cpp @@ -47,7 +47,7 @@ public: : m_interpreter(interpreter) , m_chain_node { nullptr, node } { - m_interpreter.vm().running_execution_context().current_node = &node; + m_interpreter.vm().running_execution_context().source_range = node.unrealized_source_range(); #pragma GCC diagnostic push #pragma GCC diagnostic ignored "-Wdangling-pointer" // The node pointer is popped from the interpreter in the destructor. diff --git a/Userland/Libraries/LibJS/AST.h b/Userland/Libraries/LibJS/AST.h index e0dfd28480..f851e79f7c 100644 --- a/Userland/Libraries/LibJS/AST.h +++ b/Userland/Libraries/LibJS/AST.h @@ -58,6 +58,10 @@ public: virtual void dump(int indent) const; [[nodiscard]] SourceRange source_range() const; + UnrealizedSourceRange unrealized_source_range() const + { + return { m_source_code, m_start_offset, m_end_offset }; + } u32 start_offset() const { return m_start_offset; } u32 end_offset() const { return m_end_offset; } diff --git a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp index 5dff73d7b5..0190991b9c 100644 --- a/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp +++ b/Userland/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp @@ -154,8 +154,8 @@ ThrowCompletionOr ECMAScriptFunctionObject::internal_call(Value this_argu // Non-standard callee_context.arguments.extend(move(arguments_list)); - if (auto* interpreter = vm.interpreter_if_exists()) - callee_context.current_node = interpreter->current_node(); + if (auto* interpreter = vm.interpreter_if_exists(); interpreter && interpreter->current_node()) + callee_context.source_range = interpreter->current_node()->unrealized_source_range(); // 2. Let calleeContext be PrepareForOrdinaryCall(F, undefined). // NOTE: We throw if the end of the native stack is reached, so unlike in the spec this _does_ need an exception check. @@ -225,8 +225,8 @@ ThrowCompletionOr> ECMAScriptFunctionObject::internal_const // Non-standard callee_context.arguments.extend(move(arguments_list)); - if (auto* interpreter = vm.interpreter_if_exists()) - callee_context.current_node = interpreter->current_node(); + if (auto* interpreter = vm.interpreter_if_exists(); interpreter && interpreter->current_node()) + callee_context.source_range = interpreter->current_node()->unrealized_source_range(); // 4. Let calleeContext be PrepareForOrdinaryCall(F, newTarget). // NOTE: We throw if the end of the native stack is reached, so unlike in the spec this _does_ need an exception check. diff --git a/Userland/Libraries/LibJS/Runtime/Error.cpp b/Userland/Libraries/LibJS/Runtime/Error.cpp index 2e9f907bb3..1d43647d9c 100644 --- a/Userland/Libraries/LibJS/Runtime/Error.cpp +++ b/Userland/Libraries/LibJS/Runtime/Error.cpp @@ -23,7 +23,7 @@ SourceRange const& TracebackFrame::source_range() const static auto dummy_source_code = SourceCode::create(String {}, String {}); return SourceRange { dummy_source_code, {}, {} }; } - return unrealized->source_code->range_from_offsets(unrealized->start_offset, unrealized->end_offset); + return unrealized->realize(); }(); source_range_storage = move(source_range); } @@ -85,20 +85,9 @@ void Error::populate_stack() TracebackFrame frame { .function_name = move(function_name), - .source_range_storage = TracebackFrame::UnrealizedSourceRange {}, + .source_range_storage = context->source_range, }; - // We might not have an AST node associated with the execution context, e.g. in promise - // reaction jobs (which aren't called anywhere from the source code). - // They're not going to generate any _unhandled_ exceptions though, so a meaningless - // source range is fine. - if (context->current_node) { - auto* unrealized = frame.source_range_storage.get_pointer(); - unrealized->source_code = context->current_node->source_code(); - unrealized->start_offset = context->current_node->start_offset(); - unrealized->end_offset = context->current_node->end_offset(); - } - m_traceback.append(move(frame)); } } diff --git a/Userland/Libraries/LibJS/Runtime/Error.h b/Userland/Libraries/LibJS/Runtime/Error.h index cf9315dfc8..a1f68eb8aa 100644 --- a/Userland/Libraries/LibJS/Runtime/Error.h +++ b/Userland/Libraries/LibJS/Runtime/Error.h @@ -19,11 +19,6 @@ struct TracebackFrame { DeprecatedFlyString function_name; [[nodiscard]] SourceRange const& source_range() const; - struct UnrealizedSourceRange { - u32 start_offset { 0 }; - u32 end_offset { 0 }; - RefPtr source_code; - }; mutable Variant source_range_storage; }; diff --git a/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp b/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp index e94052016e..10f53e03f2 100644 --- a/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp +++ b/Userland/Libraries/LibJS/Runtime/ExecutionContext.cpp @@ -33,7 +33,7 @@ ExecutionContext ExecutionContext::copy() const copy.lexical_environment = lexical_environment; copy.variable_environment = variable_environment; copy.private_environment = private_environment; - copy.current_node = current_node; + copy.source_range = source_range; copy.function_name = function_name; copy.this_value = this_value; copy.is_strict_mode = is_strict_mode; diff --git a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h index 1fe4b2593e..df8124816e 100644 --- a/Userland/Libraries/LibJS/Runtime/ExecutionContext.h +++ b/Userland/Libraries/LibJS/Runtime/ExecutionContext.h @@ -15,6 +15,7 @@ #include #include #include +#include namespace JS { @@ -42,7 +43,7 @@ public: // Non-standard: This points at something that owns this ExecutionContext, in case it needs to be protected from GC. GCPtr context_owner; - ASTNode const* current_node { nullptr }; + UnrealizedSourceRange source_range; DeprecatedFlyString function_name; Value this_value; MarkedVector arguments; diff --git a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp index 081fb929cc..e73741e483 100644 --- a/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp +++ b/Userland/Libraries/LibJS/Runtime/NativeFunction.cpp @@ -5,6 +5,7 @@ * SPDX-License-Identifier: BSD-2-Clause */ +#include #include #include #include @@ -141,8 +142,8 @@ ThrowCompletionOr NativeFunction::internal_call(Value this_argument, Mark // 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(); + if (auto* interpreter = vm.interpreter_if_exists(); interpreter && interpreter->current_node()) + callee_context.source_range = interpreter->current_node()->unrealized_source_range(); // -------------------------------------------------------------------------- @@ -204,8 +205,8 @@ ThrowCompletionOr> NativeFunction::internal_construct(Marke // 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(); + if (auto* interpreter = vm.interpreter_if_exists(); interpreter && interpreter->current_node()) + callee_context.source_range = interpreter->current_node()->unrealized_source_range(); // -------------------------------------------------------------------------- diff --git a/Userland/Libraries/LibJS/Runtime/VM.cpp b/Userland/Libraries/LibJS/Runtime/VM.cpp index 8aadf050a6..16fa1416c5 100644 --- a/Userland/Libraries/LibJS/Runtime/VM.cpp +++ b/Userland/Libraries/LibJS/Runtime/VM.cpp @@ -805,8 +805,8 @@ void VM::dump_backtrace() const { for (ssize_t i = m_execution_context_stack.size() - 1; i >= 0; --i) { auto& frame = m_execution_context_stack[i]; - if (frame->current_node) { - auto source_range = frame->current_node->source_range(); + if (frame->source_range.source_code) { + auto source_range = frame->source_range.realize(); dbgln("-> {} @ {}:{},{}", frame->function_name, source_range.filename(), source_range.start.line, source_range.start.column); } else { dbgln("-> {}", frame->function_name); diff --git a/Userland/Libraries/LibJS/SourceRange.h b/Userland/Libraries/LibJS/SourceRange.h index c98a6dabe8..05b1435e24 100644 --- a/Userland/Libraries/LibJS/SourceRange.h +++ b/Userland/Libraries/LibJS/SourceRange.h @@ -30,4 +30,16 @@ struct SourceRange { DeprecatedString filename() const; }; +struct UnrealizedSourceRange { + [[nodiscard]] SourceRange realize() const + { + VERIFY(source_code); + return source_code->range_from_offsets(start_offset, end_offset); + } + + RefPtr source_code; + u32 start_offset { 0 }; + u32 end_offset { 0 }; +}; + }