Spreadsheet: No longer use vm.exception() to signal exception state

Instead, use the completions which are returned directly. This means we
no longer have to worry about the global VM state when running code.

Userland/Applications/Spreadsheet/Cell.cpp

@@ -96,15 +96,18 @@ void Cell::update_data(Badge<Sheet>)
     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<Sheet>)
 
     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;
 }
 
 }

Userland/Applications/Spreadsheet/Cell.h

@@ -49,8 +49,13 @@ struct Cell : public Weakable<Cell> {
     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<JS::Value> 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<Sheet> m_sheet;
     Vector<WeakPtr<Cell>> m_referencing_cells;

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<JS::Error>(value.as_object())) {
+            auto& error = static_cast<JS::Error const&>(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();
 }

Userland/Applications/Spreadsheet/CellType/Date.cpp

@@ -23,19 +23,15 @@ DateCell::~DateCell()
 
 JS::ThrowCompletionOr<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 = 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);
-
-    return string;
+    return propagate_failure(cell, [&]() -> JS::ThrowCompletionOr<String> {
+        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);
+
+        return string;
+    });
 }
 
 JS::ThrowCompletionOr<JS::Value> DateCell::js_value(Cell& cell, const CellTypeMetadata&) const

Userland/Applications/Spreadsheet/CellType/Date.h

@@ -6,6 +6,7 @@
 
 #pragma once
 
+#include "Numeric.h"
 #include "Type.h"
 
 namespace Spreadsheet {

Userland/Applications/Spreadsheet/CellType/Numeric.cpp

@@ -23,34 +23,26 @@ NumericCell::~NumericCell()
 
 JS::ThrowCompletionOr<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 = 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);
-
-    return string;
+    return propagate_failure(cell, [&]() -> JS::ThrowCompletionOr<String> {
+        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);
+
+        return string;
+    });
 }
 
 JS::ThrowCompletionOr<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());
+    return propagate_failure(cell, [&]() {
+        return cell.js_data().to_number(cell.sheet().global_object());
+    });
 }
 
 }

Userland/Applications/Spreadsheet/CellType/Numeric.h

@@ -6,10 +6,21 @@
 
 #pragma once
 
+#include "../Cell.h"
 #include "Type.h"
 
 namespace Spreadsheet {
 
+template<typename Callable>
+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:

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());
     }
 }

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<JS::Error>(thrown_value.as_object())) {
+                    auto& error = static_cast<JS::Error const&>(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<JS::Value> 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() };
-
-    auto result = interpreter().run(script_or_error.value());
-    if (result.is_error()) {
-        auto* exc = interpreter().exception();
-        return { JS::js_undefined(), exc };
-    }
-    return { result.value(), {} };
+    if (script_or_error.is_error())
+        return interpreter().vm().throw_completion<JS::SyntaxError>(interpreter().global_object(), script_or_error.error().first().to_string());
+
+    return interpreter().run(script_or_error.value());
 }
 
 Cell* Sheet::at(StringView name)

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<JS::Value> evaluate(StringView, Cell* = nullptr);
     JS::Interpreter& interpreter() const;
     SheetGlobalObject& global_object() const { return *m_global_object; }
 

Userland/Applications/Spreadsheet/SpreadsheetModel.cpp

@@ -28,12 +28,6 @@ GUI::Variant SheetModel::data(const GUI::ModelIndex& index, GUI::ModelRole role)
             return String::empty();
 
         Function<String(JS::Value)> 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);
         }