diff --git a/Applications/Spreadsheet/Cell.cpp b/Applications/Spreadsheet/Cell.cpp index 4db2682a59..2192ce3d66 100644 --- a/Applications/Spreadsheet/Cell.cpp +++ b/Applications/Spreadsheet/Cell.cpp @@ -110,11 +110,16 @@ 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) - m_evaluated_data = m_sheet->evaluate(m_data, this); + if (!m_evaluated_externally) { + auto [value, exception] = m_sheet->evaluate(m_data, this); + m_evaluated_data = value; + m_js_exception = move(exception); + } } for (auto& ref : m_referencing_cells) { @@ -127,19 +132,25 @@ void Cell::update_data(Badge) m_evaluated_formats.background_color.clear(); m_evaluated_formats.foreground_color.clear(); - StringBuilder builder; - for (auto& fmt : m_conditional_formats) { - if (!fmt.condition.is_empty()) { - builder.clear(); - builder.append("return ("); - builder.append(fmt.condition); - builder.append(')'); - auto value = m_sheet->evaluate(builder.string_view(), this); - if (value.to_boolean()) { - if (fmt.background_color.has_value()) - m_evaluated_formats.background_color = fmt.background_color; - if (fmt.foreground_color.has_value()) - m_evaluated_formats.foreground_color = fmt.foreground_color; + if (!m_js_exception) { + StringBuilder builder; + for (auto& fmt : m_conditional_formats) { + if (!fmt.condition.is_empty()) { + builder.clear(); + builder.append("return ("); + builder.append(fmt.condition); + builder.append(')'); + auto [value, exception] = m_sheet->evaluate(builder.string_view(), this); + if (exception) { + m_js_exception = move(exception); + } else { + if (value.to_boolean()) { + if (fmt.background_color.has_value()) + m_evaluated_formats.background_color = fmt.background_color; + if (fmt.foreground_color.has_value()) + m_evaluated_formats.foreground_color = fmt.foreground_color; + } + } } } } @@ -193,6 +204,8 @@ 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; } } diff --git a/Applications/Spreadsheet/Cell.h b/Applications/Spreadsheet/Cell.h index 19e63e1328..2487a43d4e 100644 --- a/Applications/Spreadsheet/Cell.h +++ b/Applications/Spreadsheet/Cell.h @@ -67,6 +67,10 @@ struct Cell : public Weakable { void set_data(String new_data); void set_data(JS::Value new_data); 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; } const String& data() const { return m_data; } const JS::Value& evaluated_data() const { return m_evaluated_data; } @@ -117,6 +121,7 @@ private: bool m_evaluated_externally { false }; String m_data; JS::Value m_evaluated_data; + JS::Exception* m_js_exception { nullptr }; Kind m_kind { LiteralString }; WeakPtr m_sheet; Vector> m_referencing_cells; diff --git a/Applications/Spreadsheet/CellType/Date.cpp b/Applications/Spreadsheet/CellType/Date.cpp index eb0937ed55..005620006f 100644 --- a/Applications/Spreadsheet/CellType/Date.cpp +++ b/Applications/Spreadsheet/CellType/Date.cpp @@ -27,6 +27,7 @@ #include "Date.h" #include "../Cell.h" #include "../Spreadsheet.h" +#include #include namespace Spreadsheet { @@ -42,6 +43,12 @@ DateCell::~DateCell() String 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 = js_value(cell, metadata); auto string = Core::DateTime::from_timestamp(timestamp.to_i32(cell.sheet().global_object())).to_string(metadata.format.is_empty() ? "%Y-%m-%d %H:%M:%S" : metadata.format.characters()); @@ -53,7 +60,8 @@ String DateCell::display(Cell& cell, const CellTypeMetadata& metadata) const JS::Value DateCell::js_value(Cell& cell, const CellTypeMetadata&) const { - auto value = cell.js_data().to_double(cell.sheet().global_object()); + auto js_data = cell.js_data(); + auto value = js_data.to_double(cell.sheet().global_object()); return JS::Value(value / 1000); // Turn it to seconds } diff --git a/Applications/Spreadsheet/CellType/Numeric.cpp b/Applications/Spreadsheet/CellType/Numeric.cpp index b69ca1bf21..d58d48cd85 100644 --- a/Applications/Spreadsheet/CellType/Numeric.cpp +++ b/Applications/Spreadsheet/CellType/Numeric.cpp @@ -28,6 +28,7 @@ #include "../Cell.h" #include "../Spreadsheet.h" #include "Format.h" +#include namespace Spreadsheet { @@ -42,6 +43,12 @@ NumericCell::~NumericCell() String 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 = js_value(cell, metadata); String string; if (metadata.format.is_empty()) @@ -57,6 +64,12 @@ String NumericCell::display(Cell& cell, const CellTypeMetadata& metadata) const JS::Value 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()); } diff --git a/Applications/Spreadsheet/JSIntegration.cpp b/Applications/Spreadsheet/JSIntegration.cpp index 7c94cda388..5feddc9bf2 100644 --- a/Applications/Spreadsheet/JSIntegration.cpp +++ b/Applications/Spreadsheet/JSIntegration.cpp @@ -87,6 +87,16 @@ void SheetGlobalObject::initialize() define_native_function("column_index", column_index, 1); } +void SheetGlobalObject::visit_edges(Visitor& visitor) +{ + GlobalObject::visit_edges(visitor); + for (auto& it : m_sheet.cells()) { + if (it.value->exception()) + visitor.visit(it.value->exception()); + visitor.visit(it.value->evaluated_data()); + } +} + JS_DEFINE_NATIVE_FUNCTION(SheetGlobalObject::parse_cell_name) { if (vm.argument_count() != 1) { @@ -231,6 +241,13 @@ void WorkbookObject::initialize(JS::GlobalObject& global_object) define_native_function("sheet", sheet, 1); } +void WorkbookObject::visit_edges(Visitor& visitor) +{ + Base::visit_edges(visitor); + for (auto& sheet : m_workbook.sheets()) + visitor.visit(&sheet.global_object()); +} + JS_DEFINE_NATIVE_FUNCTION(WorkbookObject::sheet) { if (vm.argument_count() != 1) { diff --git a/Applications/Spreadsheet/JSIntegration.h b/Applications/Spreadsheet/JSIntegration.h index 995694d8a1..4ad289e61f 100644 --- a/Applications/Spreadsheet/JSIntegration.h +++ b/Applications/Spreadsheet/JSIntegration.h @@ -50,6 +50,7 @@ public: JS_DECLARE_NATIVE_FUNCTION(column_arithmetic); private: + virtual void visit_edges(Visitor&) override; Sheet& m_sheet; }; @@ -66,6 +67,7 @@ public: JS_DECLARE_NATIVE_FUNCTION(sheet); private: + virtual void visit_edges(Visitor&) override; Workbook& m_workbook; }; diff --git a/Applications/Spreadsheet/Spreadsheet.cpp b/Applications/Spreadsheet/Spreadsheet.cpp index 881dfb4abf..3ff03b0916 100644 --- a/Applications/Spreadsheet/Spreadsheet.cpp +++ b/Applications/Spreadsheet/Spreadsheet.cpp @@ -59,10 +59,11 @@ Sheet::Sheet(const StringView& name, Workbook& workbook) Sheet::Sheet(Workbook& workbook) : m_workbook(workbook) { + JS::DeferGC defer_gc(m_workbook.interpreter().heap()); m_global_object = m_workbook.interpreter().heap().allocate_without_global_object(*this); - m_global_object->initialize(); - m_global_object->put("workbook", m_workbook.workbook_object()); - m_global_object->put("thisSheet", m_global_object); // Self-reference is unfortunate, but required. + global_object().initialize(); + global_object().put("workbook", m_workbook.workbook_object()); + global_object().put("thisSheet", &global_object()); // Self-reference is unfortunate, but required. // Sadly, these have to be evaluated once per sheet. auto file_or_error = Core::File::open("/res/js/Spreadsheet/runtime.js", Core::IODevice::OpenMode::ReadOnly); @@ -179,26 +180,26 @@ void Sheet::update(Cell& cell) } } -JS::Value Sheet::evaluate(const StringView& source, Cell* on_behalf_of) +Sheet::ValueAndException Sheet::evaluate(const 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 parser = JS::Parser(JS::Lexer(source)); - if (parser.has_errors()) - return JS::js_undefined(); + if (parser.has_errors() || interpreter().exception()) + return { JS::js_undefined(), interpreter().exception() }; auto program = parser.parse_program(); interpreter().run(global_object(), program); if (interpreter().exception()) { - auto exc = interpreter().exception()->value(); - interpreter().vm().clear_exception(); - return exc; + auto exc = interpreter().exception(); + return { JS::js_undefined(), exc }; } auto value = interpreter().vm().last_value(); if (value.is_empty()) - return JS::js_undefined(); - return value; + return { JS::js_undefined(), {} }; + return { value, {} }; } Cell* Sheet::at(const StringView& name) diff --git a/Applications/Spreadsheet/Spreadsheet.h b/Applications/Spreadsheet/Spreadsheet.h index 23fb45d4b1..ab9ecb62d8 100644 --- a/Applications/Spreadsheet/Spreadsheet.h +++ b/Applications/Spreadsheet/Spreadsheet.h @@ -118,7 +118,11 @@ public: void update(); void update(Cell&); - JS::Value evaluate(const StringView&, Cell* = nullptr); + struct ValueAndException { + JS::Value value; + JS::Exception* exception { nullptr }; + }; + ValueAndException evaluate(const StringView&, Cell* = nullptr); JS::Interpreter& interpreter() const; SheetGlobalObject& global_object() const { return *m_global_object; } diff --git a/Applications/Spreadsheet/SpreadsheetModel.cpp b/Applications/Spreadsheet/SpreadsheetModel.cpp index 2c3826801e..84b34a5e99 100644 --- a/Applications/Spreadsheet/SpreadsheetModel.cpp +++ b/Applications/Spreadsheet/SpreadsheetModel.cpp @@ -37,16 +37,6 @@ SheetModel::~SheetModel() { } -static inline JS::Object* as_error(JS::Value value) -{ - if (value.is_object()) { - auto& object = value.as_object(); - return object.is_error() ? &object : nullptr; - } - - return nullptr; -} - GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role) const { if (!index.is_valid()) @@ -58,10 +48,22 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role) return String::empty(); if (cell->kind() == Spreadsheet::Cell::Formula) { - if (auto object = as_error(cell->evaluated_data())) { + if (auto exception = cell->exception()) { StringBuilder builder; - auto error = object->get("message").to_string_without_side_effects(); builder.append("Error: "); + auto value = exception->value(); + if (value.is_object()) { + auto& object = value.as_object(); + if (object.is_error()) { + auto error = object.get("message").to_string_without_side_effects(); + builder.append(error); + return builder.to_string(); + } + } + auto error = value.to_string(cell->sheet().global_object()); + // This is annoying, but whatever. + cell->sheet().interpreter().vm().clear_exception(); + builder.append(error); return builder.to_string(); } @@ -87,7 +89,7 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role) return {}; if (cell->kind() == Spreadsheet::Cell::Formula) { - if (as_error(cell->evaluated_data())) + if (cell->exception()) return Color(Color::Red); } diff --git a/Libraries/LibJS/AST.cpp b/Libraries/LibJS/AST.cpp index 154c3a2719..563a278f84 100644 --- a/Libraries/LibJS/AST.cpp +++ b/Libraries/LibJS/AST.cpp @@ -51,7 +51,7 @@ namespace JS { -static void update_function_name(Value value, const FlyString& name, HashTable& visited) +static void update_function_name(Value value, const FlyString& name, HashTable& visited) { if (!value.is_object()) return; @@ -73,7 +73,7 @@ static void update_function_name(Value value, const FlyString& name, HashTable visited; + HashTable visited; update_function_name(value, name, visited); } diff --git a/Libraries/LibJS/Heap/Handle.h b/Libraries/LibJS/Heap/Handle.h index e2686395fc..33fb022d21 100644 --- a/Libraries/LibJS/Heap/Handle.h +++ b/Libraries/LibJS/Heap/Handle.h @@ -65,6 +65,8 @@ public: T* cell() { return static_cast(m_impl->cell()); } const T* cell() const { return static_cast(m_impl->cell()); } + bool is_null() const { return m_impl.is_null(); } + private: explicit Handle(NonnullRefPtr impl) : m_impl(move(impl))