From 97d49cb92bab1ef353153da54b7f4cb08d65b25e Mon Sep 17 00:00:00 2001 From: Linus Groh Date: Sat, 24 Apr 2021 18:15:02 +0200 Subject: [PATCH] LibJS: Consolidate exception function names and source ranges Instead of storing the function names (in a badly named Vector) and source ranges separately, consolidate them into a new struct: TracebackFrame. This makes it both easier to use now and easier to extend in the future. Unlike before we now keep each call frame's current node source range in the traceback frame next to the function name, meaning we can display line and column numbers outside of the VM and after the call stack is emptied. --- .../Spreadsheet/CellSyntaxHighlighter.cpp | 3 ++- .../Applications/Spreadsheet/Spreadsheet.cpp | 11 ++++++---- .../Libraries/LibJS/Runtime/Exception.cpp | 22 +++++++------------ Userland/Libraries/LibJS/Runtime/Exception.h | 15 ++++++++----- Userland/Utilities/js.cpp | 21 ++++++++++-------- 5 files changed, 39 insertions(+), 33 deletions(-) diff --git a/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp b/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp index 826f3f1ccf..4e74b7b6ae 100644 --- a/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp +++ b/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp @@ -35,7 +35,8 @@ void CellSyntaxHighlighter::rehighlight(const Palette& palette) false); if (m_cell && m_cell->exception()) { - auto range = m_cell->exception()->source_ranges().first(); + auto& traceback = m_cell->exception()->traceback(); + auto& range = traceback.first().source_range; GUI::TextRange text_range { { range.start.line - 1, range.start.column }, { range.end.line - 1, range.end.column - 1 } }; m_client->spans().prepend( GUI::TextDocumentSpan { diff --git a/Userland/Applications/Spreadsheet/Spreadsheet.cpp b/Userland/Applications/Spreadsheet/Spreadsheet.cpp index 30db02f98b..897d011313 100644 --- a/Userland/Applications/Spreadsheet/Spreadsheet.cpp +++ b/Userland/Applications/Spreadsheet/Spreadsheet.cpp @@ -55,10 +55,13 @@ Sheet::Sheet(Workbook& workbook) parser.print_errors(); } else { interpreter().run(global_object(), parser.parse_program()); - if (auto exc = interpreter().exception()) { - warnln("Spreadsheet: Failed to run runtime code: "); - for (auto& t : exc->trace()) - warnln("{}", t); + if (auto* exception = interpreter().exception()) { + warnln("Spreadsheet: Failed to run runtime code:"); + for (auto& traceback_frame : exception->traceback()) { + auto& function_name = traceback_frame.function_name; + auto& source_range = traceback_frame.source_range; + dbgln(" {} at {}:{}:{}", function_name, source_range.filename, source_range.start.line, source_range.start.column); + } interpreter().vm().clear_exception(); } } diff --git a/Userland/Libraries/LibJS/Runtime/Exception.cpp b/Userland/Libraries/LibJS/Runtime/Exception.cpp index d59b628d07..ba27919014 100644 --- a/Userland/Libraries/LibJS/Runtime/Exception.cpp +++ b/Userland/Libraries/LibJS/Runtime/Exception.cpp @@ -1,5 +1,6 @@ /* * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2021, Linus Groh * * SPDX-License-Identifier: BSD-2-Clause */ @@ -16,23 +17,16 @@ Exception::Exception(Value value) : m_value(value) { auto& vm = this->vm(); - auto& call_stack = vm.call_stack(); - for (ssize_t i = call_stack.size() - 1; i >= 0; --i) { - String function_name = call_stack[i]->function_name; + m_traceback.ensure_capacity(vm.call_stack().size()); + for (auto* call_frame : vm.call_stack()) { + auto function_name = call_frame->function_name; if (function_name.is_empty()) function_name = ""; - m_trace.append(function_name); + m_traceback.prepend({ + .function_name = move(function_name), + .source_range = call_frame->current_node->source_range(), + }); } - - if (auto* interpreter = vm.interpreter_if_exists()) { - for (auto* node_chain = interpreter->executing_ast_node_chain(); node_chain; node_chain = node_chain->previous) { - m_source_ranges.append(node_chain->node.source_range()); - } - } -} - -Exception::~Exception() -{ } void Exception::visit_edges(Visitor& visitor) diff --git a/Userland/Libraries/LibJS/Runtime/Exception.h b/Userland/Libraries/LibJS/Runtime/Exception.h index bb9037db95..1065ecb115 100644 --- a/Userland/Libraries/LibJS/Runtime/Exception.h +++ b/Userland/Libraries/LibJS/Runtime/Exception.h @@ -1,11 +1,13 @@ /* * Copyright (c) 2020, Andreas Kling + * Copyright (c) 2021, Linus Groh * * SPDX-License-Identifier: BSD-2-Clause */ #pragma once +#include #include #include #include @@ -13,22 +15,25 @@ namespace JS { +struct TracebackFrame { + FlyString function_name; + SourceRange source_range; +}; + class Exception : public Cell { public: explicit Exception(Value); - virtual ~Exception() override; + virtual ~Exception() override = default; Value value() const { return m_value; } - const Vector& trace() const { return m_trace; } - const Vector& source_ranges() const { return m_source_ranges; } + const Vector& traceback() const { return m_traceback; } private: virtual const char* class_name() const override { return "Exception"; } virtual void visit_edges(Visitor&) override; Value m_value; - Vector m_trace; - Vector m_source_ranges; + Vector m_traceback; }; } diff --git a/Userland/Utilities/js.cpp b/Userland/Utilities/js.cpp index 391520e789..b2f2ed09fe 100644 --- a/Userland/Utilities/js.cpp +++ b/Userland/Utilities/js.cpp @@ -489,24 +489,27 @@ static bool parse_and_run(JS::Interpreter& interpreter, const StringView& source vm->clear_exception(); out("Uncaught exception: "); print(exception->value()); - auto& trace = exception->trace(); - if (trace.size() > 1) { + auto& traceback = exception->traceback(); + if (traceback.size() > 1) { unsigned repetitions = 0; - for (size_t i = 0; i < trace.size(); ++i) { - auto& function_name = trace[i]; - if (i + 1 < trace.size() && trace[i + 1] == function_name) { - repetitions++; - continue; + for (size_t i = 0; i < traceback.size(); ++i) { + auto& traceback_frame = traceback[i]; + if (i + 1 < traceback.size()) { + auto& next_traceback_frame = traceback[i + 1]; + if (next_traceback_frame.function_name == traceback_frame.function_name) { + repetitions++; + continue; + } } if (repetitions > 4) { // If more than 5 (1 + >4) consecutive function calls with the same name, print // the name only once and show the number of repetitions instead. This prevents // printing ridiculously large call stacks of recursive functions. - outln(" -> {}", function_name); + outln(" -> {}", traceback_frame.function_name); outln(" {} more calls", repetitions); } else { for (size_t j = 0; j < repetitions + 1; ++j) - outln(" -> {}", function_name); + outln(" -> {}", traceback_frame.function_name); } repetitions = 0; }