LibJS: Convert Interpreter::run() to ThrowCompletionOr<Value>

Instead of making it a void function, checking for an exception, and
then receiving the relevant result via VM::last_value(), we can
consolidate all of this by using completions.

This allows us to remove more uses of VM::exception(), and all uses of
VM::last_value().

Meta/Lagom/Fuzzers/FuzzJs.cpp

@@ -21,7 +21,7 @@ extern "C" int LLVMFuzzerTestOneInput(const uint8_t* data, size_t size)
     if (!parser.has_errors()) {
         auto vm = JS::VM::create();
         auto interpreter = JS::Interpreter::create<JS::GlobalObject>(*vm);
-        interpreter->run(interpreter->global_object(), *program);
+        (void)interpreter->run(interpreter->global_object(), *program);
     }
     return 0;
 }

Meta/Lagom/Fuzzers/FuzzilliJs.cpp

@@ -213,8 +213,8 @@ int main(int, char**)
         if (parser.has_errors()) {
             result = 1;
         } else {
-            interpreter->run(interpreter->global_object(), *program);
+            auto completion = interpreter->run(interpreter->global_object(), *program);
-            if (interpreter->exception()) {
+            if (completion.is_error()) {
                 result = 1;
                 vm->clear_exception();
             }

Userland/Applications/Assistant/Providers.cpp

@@ -98,11 +98,11 @@ void CalculatorProvider::query(String const& query, Function<void(NonnullRefPtrV
     if (parser.has_errors())
         return;
 
-    interpreter->run(interpreter->global_object(), *program);
+    auto completion = interpreter->run(interpreter->global_object(), *program);
-    if (interpreter->exception())
+    if (completion.is_error())
         return;
 
-    auto result = interpreter->vm().last_value();
+    auto result = completion.release_value();
     String calculation;
     if (!result.is_number()) {
         calculation = "0";

Userland/Applications/Spreadsheet/Spreadsheet.cpp

@@ -55,7 +55,7 @@ Sheet::Sheet(Workbook& workbook)
             warnln("Spreadsheet: Failed to parse runtime code");
             parser.print_errors();
         } else {
-            interpreter().run(global_object(), parser.parse_program());
+            (void)interpreter().run(global_object(), parser.parse_program());
             if (auto* exception = interpreter().exception()) {
                 warnln("Spreadsheet: Failed to run runtime code:");
                 for (auto& traceback_frame : exception->traceback()) {
@@ -164,16 +164,12 @@ Sheet::ValueAndException Sheet::evaluate(StringView source, Cell* on_behalf_of)
     if (parser.has_errors() || interpreter().exception())
         return { JS::js_undefined(), interpreter().exception() };
 
-    interpreter().run(global_object(), program);
+    auto result = interpreter().run(global_object(), program);
-    if (interpreter().exception()) {
+    if (result.is_error()) {
         auto exc = interpreter().exception();
         return { JS::js_undefined(), exc };
     }
-
+    return { result.value(), {} };
-    auto value = interpreter().vm().last_value();
-    if (value.is_empty())
-        return { JS::js_undefined(), {} };
-    return { value, {} };
 }
 
 Cell* Sheet::at(StringView name)

Userland/DevTools/HackStudio/Debugger/EvaluateExpressionDialog.cpp

@@ -112,21 +112,21 @@ void EvaluateExpressionDialog::handle_evaluation(const String& expression)
     auto program = parser.parse_program();
 
     StringBuilder output_html;
+    auto result = JS::ThrowCompletionOr<JS::Value> { JS::js_undefined() };
     if (parser.has_errors()) {
         auto error = parser.errors()[0];
         auto hint = error.source_location_hint(expression);
         if (!hint.is_empty())
             output_html.append(String::formatted("<pre>{}</pre>", escape_html_entities(hint)));
-        m_interpreter->vm().throw_exception<JS::SyntaxError>(m_interpreter->global_object(), error.to_string());
+        result = m_interpreter->vm().throw_completion<JS::SyntaxError>(m_interpreter->global_object(), error.to_string());
     } else {
-        m_interpreter->run(m_interpreter->global_object(), *program);
+        result = m_interpreter->run(m_interpreter->global_object(), *program);
     }
 
-    if (m_interpreter->exception()) {
+    if (result.is_error()) {
-        auto* exception = m_interpreter->exception();
         m_interpreter->vm().clear_exception();
         output_html.append("Uncaught exception: ");
-        auto error = exception->value();
+        auto error = *result.throw_completion().value();
         if (error.is_object())
             output_html.append(JS::MarkupGenerator::html_from_error(error.as_object()));
         else
@@ -135,7 +135,7 @@ void EvaluateExpressionDialog::handle_evaluation(const String& expression)
         return;
     }
 
-    set_output(JS::MarkupGenerator::html_from_value(m_interpreter->vm().last_value()));
+    set_output(JS::MarkupGenerator::html_from_value(result.value()));
 }
 
 void EvaluateExpressionDialog::set_output(StringView html)

Userland/Libraries/LibJS/Interpreter.cpp

@@ -38,7 +38,7 @@ Interpreter::~Interpreter()
 {
 }
 
-void Interpreter::run(GlobalObject& global_object, const Program& program)
+ThrowCompletionOr<Value> Interpreter::run(GlobalObject& global_object, const Program& program)
 {
     // FIXME: Why does this receive a GlobalObject? Interpreter has one already, and this might not be in sync with the Realm's GlobalObject.
 
@@ -71,6 +71,12 @@ void Interpreter::run(GlobalObject& global_object, const Program& program)
     vm.pop_execution_context();
 
     vm.finish_execution_generation();
+
+    if (completion.is_abrupt()) {
+        VERIFY(completion.type() == Completion::Type::Throw);
+        return completion.release_error();
+    }
+    return completion.value().value_or(js_undefined());
 }
 
 GlobalObject& Interpreter::global_object()

Userland/Libraries/LibJS/Interpreter.h

@@ -15,6 +15,7 @@
 #include <LibJS/Forward.h>
 #include <LibJS/Heap/DeferGC.h>
 #include <LibJS/Heap/Heap.h>
+#include <LibJS/Runtime/Completion.h>
 #include <LibJS/Runtime/DeclarativeEnvironment.h>
 #include <LibJS/Runtime/ErrorTypes.h>
 #include <LibJS/Runtime/Exception.h>
@@ -52,7 +53,7 @@ public:
 
     ~Interpreter();
 
-    void run(GlobalObject&, const Program&);
+    ThrowCompletionOr<Value> run(GlobalObject&, const Program&);
 
     GlobalObject& global_object();
     const GlobalObject& global_object() const;

Userland/Libraries/LibTest/JavaScriptTestRunner.h

@@ -342,11 +342,9 @@ inline JSFileResult TestRunner::run_file_test(const String& test_path)
         JS::Bytecode::Interpreter bytecode_interpreter(interpreter->global_object(), interpreter->realm());
         MUST(bytecode_interpreter.run(executable));
     } else {
-        interpreter->run(interpreter->global_object(), m_test_script->parse_node());
+        MUST(interpreter->run(interpreter->global_object(), m_test_script->parse_node()));
     }
 
-    VERIFY(!g_vm->exception());
-
     auto file_script = parse_script(test_path, interpreter->realm());
     if (file_script.is_error())
         return { test_path, file_script.error() };
@@ -358,7 +356,7 @@ inline JSFileResult TestRunner::run_file_test(const String& test_path)
         JS::Bytecode::Interpreter bytecode_interpreter(interpreter->global_object(), interpreter->realm());
         (void)bytecode_interpreter.run(executable);
     } else {
-        interpreter->run(interpreter->global_object(), file_script.value()->parse_node());
+        (void)interpreter->run(interpreter->global_object(), file_script.value()->parse_node());
     }
 
     if (g_vm->exception())

Userland/Libraries/LibWeb/DOM/Document.cpp

@@ -687,10 +687,13 @@ JS::Value Document::run_javascript(StringView source, StringView filename)
     }
     auto& interpreter = document().interpreter();
     auto& vm = interpreter.vm();
-    interpreter.run(interpreter.global_object(), *program);
+    auto result = interpreter.run(interpreter.global_object(), *program);
-    if (vm.exception())
+    if (result.is_error()) {
+        // FIXME: I'm sure the spec could tell us something about error propagation here!
         vm.clear_exception();
-    return vm.last_value();
+        return {};
+    }
+    return result.value();
 }
 
 // https://dom.spec.whatwg.org/#dom-document-createelement

Userland/Libraries/LibWeb/HTML/Scripting/ClassicScript.cpp

@@ -70,12 +70,15 @@ JS::Value ClassicScript::run(RethrowErrors rethrow_errors)
 
     auto timer = Core::ElapsedTimer::start_new();
     auto interpreter = JS::Interpreter::create_with_existing_realm(m_script_record->realm());
-    interpreter->run(interpreter->global_object(), m_script_record->parse_node());
+
-    auto& vm = interpreter->vm();
+    auto result = interpreter->run(interpreter->global_object(), m_script_record->parse_node());
-    if (vm.exception())
-        vm.clear_exception();
     dbgln("ClassicScript: Finished running script {}, Duration: {}ms", filename(), timer.elapsed());
-    return vm.last_value();
+    if (result.is_error()) {
+        // FIXME: Propagate error according to the spec.
+        interpreter->vm().clear_exception();
+        return {};
+    }
+    return result.value();
 }
 
 ClassicScript::ClassicScript(AK::URL base_url, String filename)

Userland/Services/WebContent/ClientConnection.cpp

@@ -331,9 +331,9 @@ void ClientConnection::run_javascript(String const& js_source)
 
     auto parser = JS::Parser(JS::Lexer(js_source));
     auto program = parser.parse_program();
-    interpreter.run(interpreter.global_object(), *program);
+    auto result = interpreter.run(interpreter.global_object(), *program);
 
-    if (interpreter.vm().exception()) {
+    if (result.is_error()) {
         dbgln("Exception :(");
         interpreter.vm().clear_exception();
     }

Userland/Services/WebContent/WebContentConsoleClient.cpp

@@ -32,12 +32,13 @@ void WebContentConsoleClient::handle_input(String const& js_source)
     auto program = parser.parse_program();
 
     StringBuilder output_html;
+    auto result = JS::ThrowCompletionOr<JS::Value> { JS::js_undefined() };
     if (parser.has_errors()) {
         auto error = parser.errors()[0];
         auto hint = error.source_location_hint(js_source);
         if (!hint.is_empty())
             output_html.append(String::formatted("<pre>{}</pre>", escape_html_entities(hint)));
-        m_interpreter->vm().throw_exception<JS::SyntaxError>(*m_console_global_object.cell(), error.to_string());
+        result = m_interpreter->vm().throw_completion<JS::SyntaxError>(*m_console_global_object.cell(), error.to_string());
     } else {
         // FIXME: This is not the correct way to do this, we probably want to have
         //        multiple execution contexts we switch between.
@@ -46,16 +47,15 @@ void WebContentConsoleClient::handle_input(String const& js_source)
         auto& this_value_before = m_interpreter->realm().global_environment().global_this_value();
         m_interpreter->realm().set_global_object(*m_console_global_object.cell(), &global_object_before);
 
-        m_interpreter->run(*m_console_global_object.cell(), *program);
+        result = m_interpreter->run(*m_console_global_object.cell(), *program);
 
         m_interpreter->realm().set_global_object(global_object_before, &this_value_before);
     }
 
-    if (m_interpreter->exception()) {
+    if (result.is_error()) {
-        auto* exception = m_interpreter->exception();
         m_interpreter->vm().clear_exception();
         output_html.append("Uncaught exception: ");
-        auto error = exception->value();
+        auto error = *result.throw_completion().value();
         if (error.is_object())
             output_html.append(JS::MarkupGenerator::html_from_error(error.as_object()));
         else
@@ -64,7 +64,7 @@ void WebContentConsoleClient::handle_input(String const& js_source)
         return;
     }
 
-    print_html(JS::MarkupGenerator::html_from_value(m_interpreter->vm().last_value()));
+    print_html(JS::MarkupGenerator::html_from_value(result.value()));
 }
 
 void WebContentConsoleClient::print_html(String const& line)

Userland/Utilities/js.cpp

@@ -915,6 +915,8 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin
     if (s_dump_ast)
         program->dump(0);
 
+    auto result = JS::ThrowCompletionOr<JS::Value> { JS::js_undefined() };
+
     if (parser.has_errors()) {
         auto error = parser.errors()[0];
         if (!s_disable_source_location_hints) {
@@ -922,7 +924,7 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin
             if (!hint.is_empty())
                 js_outln("{}", hint);
         }
-        vm->throw_exception<JS::SyntaxError>(interpreter.global_object(), error.to_string());
+        result = vm->throw_completion<JS::SyntaxError>(interpreter.global_object(), error.to_string());
     } else {
         if (JS::Bytecode::g_dump_bytecode || s_run_bytecode) {
             auto executable = JS::Bytecode::Generator::generate(*program);
@@ -938,15 +940,12 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin
 
             if (s_run_bytecode) {
                 JS::Bytecode::Interpreter bytecode_interpreter(interpreter.global_object(), interpreter.realm());
-                auto result = bytecode_interpreter.run(executable);
+                result = bytecode_interpreter.run(executable);
-                // Since all the error handling code uses vm.exception() we just rethrow any exception we got here.
-                if (result.is_error())
-                    vm->throw_exception(interpreter.global_object(), *result.throw_completion().value());
             } else {
                 return true;
             }
         } else {
-            interpreter.run(interpreter.global_object(), *program);
+            result = interpreter.run(interpreter.global_object(), *program);
         }
     }
 
@@ -982,15 +981,15 @@ static bool parse_and_run(JS::Interpreter& interpreter, StringView source, Strin
         }
     };
 
-    if (vm->exception()) {
+    if (result.is_error()) {
-        handle_exception();
-        return false;
-    }
-    if (s_print_last_result)
-        print(vm->last_value());
-    if (vm->exception()) {
         handle_exception();
         return false;
+    } else if (s_print_last_result) {
+        print(result.value());
+        if (vm->exception()) {
+            handle_exception();
+            return false;
+        }
     }
     return true;
 }
@@ -1010,7 +1009,7 @@ static JS::ThrowCompletionOr<JS::Value> load_file_impl(JS::VM& vm, JS::GlobalObj
         return vm.throw_completion<JS::SyntaxError>(global_object, error.to_string());
     }
     // FIXME: Use eval()-like semantics and execute in current scope?
-    vm.interpreter().run(global_object, *program);
+    TRY(vm.interpreter().run(global_object, *program));
     return JS::js_undefined();
 }