diff --git a/Userland/Applications/Spreadsheet/Cell.cpp b/Userland/Applications/Spreadsheet/Cell.cpp index 9a1a4816c2..32e98270e2 100644 --- a/Userland/Applications/Spreadsheet/Cell.cpp +++ b/Userland/Applications/Spreadsheet/Cell.cpp @@ -96,15 +96,18 @@ void Cell::update_data(Badge) if (!m_dirty) return; - m_js_exception = {}; - if (m_dirty) { m_dirty = false; if (m_kind == Formula) { if (!m_evaluated_externally) { - auto [value, exception] = m_sheet->evaluate(m_data, this); - m_evaluated_data = value; - m_js_exception = move(exception); + auto value_or_error = m_sheet->evaluate(m_data, this); + if (value_or_error.is_error()) { + m_evaluated_data = JS::js_undefined(); + m_thrown_value = *value_or_error.release_error().release_value(); + } else { + m_evaluated_data = value_or_error.release_value(); + m_thrown_value = {}; + } } } @@ -118,14 +121,14 @@ void Cell::update_data(Badge) m_evaluated_formats.background_color.clear(); m_evaluated_formats.foreground_color.clear(); - if (!m_js_exception) { + if (m_thrown_value.is_empty()) { for (auto& fmt : m_conditional_formats) { if (!fmt.condition.is_empty()) { - auto [value, exception] = m_sheet->evaluate(fmt.condition, this); - if (exception) { - m_js_exception = move(exception); + auto value_or_error = m_sheet->evaluate(fmt.condition, this); + if (value_or_error.is_error()) { + m_thrown_value = *value_or_error.release_error().release_value(); } else { - if (value.to_boolean()) { + if (value_or_error.release_value().to_boolean()) { if (fmt.background_color.has_value()) m_evaluated_formats.background_color = fmt.background_color; if (fmt.foreground_color.has_value()) @@ -185,8 +188,7 @@ void Cell::copy_from(const Cell& other) m_type_metadata = other.m_type_metadata; m_conditional_formats = other.m_conditional_formats; m_evaluated_formats = other.m_evaluated_formats; - if (!other.m_js_exception) - m_js_exception = other.m_js_exception; + m_thrown_value = other.m_thrown_value; } } diff --git a/Userland/Applications/Spreadsheet/Cell.h b/Userland/Applications/Spreadsheet/Cell.h index 1e8830084e..231bb92738 100644 --- a/Userland/Applications/Spreadsheet/Cell.h +++ b/Userland/Applications/Spreadsheet/Cell.h @@ -49,8 +49,13 @@ struct Cell : public Weakable { bool dirty() const { return m_dirty; } void clear_dirty() { m_dirty = false; } - void set_exception(JS::Exception* exc) { m_js_exception = exc; } - JS::Exception* exception() const { return m_js_exception; } + void set_thrown_value(JS::Value value) { m_thrown_value = value; } + Optional thrown_value() const + { + if (m_thrown_value.is_empty()) + return {}; + return m_thrown_value; + } const String& data() const { return m_data; } const JS::Value& evaluated_data() const { return m_evaluated_data; } @@ -103,7 +108,7 @@ private: bool m_evaluated_externally { false }; String m_data; JS::Value m_evaluated_data; - JS::Exception* m_js_exception { nullptr }; + JS::Value m_thrown_value; Kind m_kind { LiteralString }; WeakPtr m_sheet; Vector> m_referencing_cells; diff --git a/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp b/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp index 533d17d108..c6bdde6bea 100644 --- a/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp +++ b/Userland/Applications/Spreadsheet/CellSyntaxHighlighter.cpp @@ -34,21 +34,24 @@ void CellSyntaxHighlighter::rehighlight(const Palette& palette) (u64)-1, false); - if (m_cell && m_cell->exception()) { - 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 } }; - m_client->spans().prepend( - GUI::TextDocumentSpan { - text_range, - Gfx::TextAttributes { - Color::Black, - Color::Red, - false, - false, - }, - (u64)-1, - false }); + if (m_cell && m_cell->thrown_value().has_value()) { + if (auto value = m_cell->thrown_value().value(); value.is_object() && is(value.as_object())) { + auto& error = static_cast(value.as_object()); + auto& traceback = error.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 } }; + m_client->spans().prepend( + GUI::TextDocumentSpan { + text_range, + Gfx::TextAttributes { + Color::Black, + Color::Red, + false, + false, + }, + (u64)-1, + false }); + } } m_client->do_update(); } diff --git a/Userland/Applications/Spreadsheet/CellType/Date.cpp b/Userland/Applications/Spreadsheet/CellType/Date.cpp index 9714ebb47d..65ce76d6a9 100644 --- a/Userland/Applications/Spreadsheet/CellType/Date.cpp +++ b/Userland/Applications/Spreadsheet/CellType/Date.cpp @@ -23,19 +23,15 @@ DateCell::~DateCell() JS::ThrowCompletionOr DateCell::display(Cell& cell, const CellTypeMetadata& metadata) const { - ScopeGuard propagate_exception { [&cell] { - if (auto exc = cell.sheet().interpreter().exception()) { - cell.sheet().interpreter().vm().clear_exception(); - cell.set_exception(exc); - } - } }; - auto timestamp = TRY(js_value(cell, metadata)); - auto string = Core::DateTime::from_timestamp(TRY(timestamp.to_i32(cell.sheet().global_object()))).to_string(metadata.format.is_empty() ? "%Y-%m-%d %H:%M:%S" : metadata.format.characters()); + return propagate_failure(cell, [&]() -> JS::ThrowCompletionOr { + auto timestamp = TRY(js_value(cell, metadata)); + auto string = Core::DateTime::from_timestamp(TRY(timestamp.to_i32(cell.sheet().global_object()))).to_string(metadata.format.is_empty() ? "%Y-%m-%d %H:%M:%S" : metadata.format.characters()); - if (metadata.length >= 0) - return string.substring(0, metadata.length); + if (metadata.length >= 0) + return string.substring(0, metadata.length); - return string; + return string; + }); } JS::ThrowCompletionOr DateCell::js_value(Cell& cell, const CellTypeMetadata&) const diff --git a/Userland/Applications/Spreadsheet/CellType/Date.h b/Userland/Applications/Spreadsheet/CellType/Date.h index b39583c371..41f6b3ec46 100644 --- a/Userland/Applications/Spreadsheet/CellType/Date.h +++ b/Userland/Applications/Spreadsheet/CellType/Date.h @@ -6,6 +6,7 @@ #pragma once +#include "Numeric.h" #include "Type.h" namespace Spreadsheet { diff --git a/Userland/Applications/Spreadsheet/CellType/Numeric.cpp b/Userland/Applications/Spreadsheet/CellType/Numeric.cpp index 9ebe1d8aa9..186eea277e 100644 --- a/Userland/Applications/Spreadsheet/CellType/Numeric.cpp +++ b/Userland/Applications/Spreadsheet/CellType/Numeric.cpp @@ -23,34 +23,26 @@ NumericCell::~NumericCell() JS::ThrowCompletionOr NumericCell::display(Cell& cell, const CellTypeMetadata& metadata) const { - ScopeGuard propagate_exception { [&cell] { - if (auto exc = cell.sheet().interpreter().exception()) { - cell.sheet().interpreter().vm().clear_exception(); - cell.set_exception(exc); - } - } }; - auto value = TRY(js_value(cell, metadata)); - String string; - if (metadata.format.is_empty()) - string = TRY(value.to_string(cell.sheet().global_object())); - else - string = format_double(metadata.format.characters(), TRY(value.to_double(cell.sheet().global_object()))); + return propagate_failure(cell, [&]() -> JS::ThrowCompletionOr { + auto value = TRY(js_value(cell, metadata)); + String string; + if (metadata.format.is_empty()) + string = TRY(value.to_string(cell.sheet().global_object())); + else + string = format_double(metadata.format.characters(), TRY(value.to_double(cell.sheet().global_object()))); - if (metadata.length >= 0) - return string.substring(0, metadata.length); + if (metadata.length >= 0) + return string.substring(0, metadata.length); - return string; + return string; + }); } JS::ThrowCompletionOr NumericCell::js_value(Cell& cell, const CellTypeMetadata&) const { - ScopeGuard propagate_exception { [&cell] { - if (auto exc = cell.sheet().interpreter().exception()) { - cell.sheet().interpreter().vm().clear_exception(); - cell.set_exception(exc); - } - } }; - return cell.js_data().to_number(cell.sheet().global_object()); + return propagate_failure(cell, [&]() { + return cell.js_data().to_number(cell.sheet().global_object()); + }); } } diff --git a/Userland/Applications/Spreadsheet/CellType/Numeric.h b/Userland/Applications/Spreadsheet/CellType/Numeric.h index ae49204e3d..928bdf6d81 100644 --- a/Userland/Applications/Spreadsheet/CellType/Numeric.h +++ b/Userland/Applications/Spreadsheet/CellType/Numeric.h @@ -6,10 +6,21 @@ #pragma once +#include "../Cell.h" #include "Type.h" namespace Spreadsheet { +template +static auto propagate_failure(Cell& cell, Callable&& steps) +{ + auto result_or_error = steps(); + if (result_or_error.is_error()) + cell.set_thrown_value(*result_or_error.throw_completion().value()); + + return result_or_error; +} + class NumericCell : public CellType { public: diff --git a/Userland/Applications/Spreadsheet/JSIntegration.cpp b/Userland/Applications/Spreadsheet/JSIntegration.cpp index 5ddc2ffb46..d4dda776aa 100644 --- a/Userland/Applications/Spreadsheet/JSIntegration.cpp +++ b/Userland/Applications/Spreadsheet/JSIntegration.cpp @@ -164,8 +164,9 @@ void SheetGlobalObject::visit_edges(Visitor& visitor) { Base::visit_edges(visitor); for (auto& it : m_sheet.cells()) { - if (it.value->exception()) - visitor.visit(it.value->exception()); + if (auto opt_thrown_value = it.value->thrown_value(); opt_thrown_value.has_value()) + visitor.visit(*opt_thrown_value); + visitor.visit(it.value->evaluated_data()); } } diff --git a/Userland/Applications/Spreadsheet/Spreadsheet.cpp b/Userland/Applications/Spreadsheet/Spreadsheet.cpp index cc0a3721b8..4c28433d0e 100644 --- a/Userland/Applications/Spreadsheet/Spreadsheet.cpp +++ b/Userland/Applications/Spreadsheet/Spreadsheet.cpp @@ -60,15 +60,22 @@ Sheet::Sheet(Workbook& workbook) warnln("SyntaxError: {}", error.to_string()); } } else { - (void)interpreter().run(script_or_error.value()); - if (auto* exception = interpreter().exception()) { + auto result = interpreter().run(script_or_error.value()); + if (result.is_error()) { 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); + auto thrown_value = *result.throw_completion().value(); + warn("Threw: {}", thrown_value.to_string_without_side_effects()); + if (thrown_value.is_object() && is(thrown_value.as_object())) { + auto& error = static_cast(thrown_value.as_object()); + warnln(" with message '{}'", error.get_without_side_effects(interpreter().vm().names.message)); + for (auto& traceback_frame : error.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); + } + } else { + warnln(); } - interpreter().vm().clear_exception(); } } } @@ -159,22 +166,15 @@ void Sheet::update(Cell& cell) } } -Sheet::ValueAndException Sheet::evaluate(StringView source, Cell* on_behalf_of) +JS::ThrowCompletionOr Sheet::evaluate(StringView source, Cell* on_behalf_of) { TemporaryChange cell_change { m_current_cell_being_evaluated, on_behalf_of }; - ScopeGuard clear_exception { [&] { interpreter().vm().clear_exception(); } }; auto script_or_error = JS::Script::parse(source, interpreter().realm()); - // FIXME: Convert parser errors to exceptions to show them to the user. - if (script_or_error.is_error() || interpreter().exception()) - return { JS::js_undefined(), interpreter().exception() }; + if (script_or_error.is_error()) + return interpreter().vm().throw_completion(interpreter().global_object(), script_or_error.error().first().to_string()); - auto result = interpreter().run(script_or_error.value()); - if (result.is_error()) { - auto* exc = interpreter().exception(); - return { JS::js_undefined(), exc }; - } - return { result.value(), {} }; + return interpreter().run(script_or_error.value()); } Cell* Sheet::at(StringView name) diff --git a/Userland/Applications/Spreadsheet/Spreadsheet.h b/Userland/Applications/Spreadsheet/Spreadsheet.h index 213131185d..0d4a36c5e1 100644 --- a/Userland/Applications/Spreadsheet/Spreadsheet.h +++ b/Userland/Applications/Spreadsheet/Spreadsheet.h @@ -107,11 +107,7 @@ public: } } - struct ValueAndException { - JS::Value value; - JS::Exception* exception { nullptr }; - }; - ValueAndException evaluate(StringView, Cell* = nullptr); + JS::ThrowCompletionOr evaluate(StringView, Cell* = nullptr); JS::Interpreter& interpreter() const; SheetGlobalObject& global_object() const { return *m_global_object; } diff --git a/Userland/Applications/Spreadsheet/SpreadsheetModel.cpp b/Userland/Applications/Spreadsheet/SpreadsheetModel.cpp index 8be1f77dc7..8d97b57516 100644 --- a/Userland/Applications/Spreadsheet/SpreadsheetModel.cpp +++ b/Userland/Applications/Spreadsheet/SpreadsheetModel.cpp @@ -28,12 +28,6 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role) return String::empty(); Function to_string_as_exception = [&](JS::Value value) { - ScopeGuard clear_exception { - [&] { - cell->sheet().interpreter().vm().clear_exception(); - } - }; - StringBuilder builder; builder.append("Error: "sv); if (value.is_object()) { @@ -58,8 +52,8 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role) }; if (cell->kind() == Spreadsheet::Cell::Formula) { - if (auto exception = cell->exception()) - return to_string_as_exception(exception->value()); + if (auto opt_throw_value = cell->thrown_value(); opt_throw_value.has_value()) + return to_string_as_exception(*opt_throw_value); } auto display = cell->typed_display(); @@ -86,7 +80,7 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role) return {}; if (cell->kind() == Spreadsheet::Cell::Formula) { - if (cell->exception()) + if (cell->thrown_value().has_value()) return Color(Color::Red); }