LibJS: Make run_executable() return simple ThrowCompletionOr<Value>

awesomekling · awesomekling · commit 5706831328c4 · 2025-10-31T08:56:02.000+01:00
We don't need to return two values; running an executable only ever
produces a throw completion, or a normal completion, i.e a Value.

This necessitated a few minor changes, such as adding a way to check
if a JS::Cell is a GeneratorResult.
diff --git a/Libraries/LibJS/Bytecode/Interpreter.cpp b/Libraries/LibJS/Bytecode/Interpreter.cpp
@@ -256,12 +256,7 @@ ThrowCompletionOr<Value> Interpreter::run(Script& script_record, GC::Ptr<Environ
     // 13. If result.[[Type]] is normal, then
     if (executable) {
         // a. Set result to Completion(Evaluation of script).
-        auto result_or_error = run_executable(*script_context, *executable, {}, {});
-        if (result_or_error.value.is_error())
-            result = result_or_error.value.release_error();
-        else {
-            result = result_or_error.return_register_value.is_special_empty_value() ? normal_completion(js_undefined()) : result_or_error.return_register_value;
-        }
+        result = run_executable(*script_context, *executable, {}, {});
 
         // b. If result is a normal completion and result.[[Value]] is empty, then
         if (result.type() == Completion::Type::Normal && result.value().is_special_empty_value()) {
@@ -707,7 +702,7 @@ Utf16FlyString const& Interpreter::get_identifier(IdentifierTableIndex index) co
     return m_running_execution_context->identifier_table.data()[index.value];
 }
 
-Interpreter::ResultAndReturnRegister Interpreter::run_executable(ExecutionContext& context, Executable& executable, Optional<size_t> entry_point, Value initial_accumulator_value)
+ThrowCompletionOr<Value> Interpreter::run_executable(ExecutionContext& context, Executable& executable, Optional<size_t> entry_point, Value initial_accumulator_value)
 {
     dbgln_if(JS_BYTECODE_DEBUG, "Bytecode::Interpreter will run unit {}", &executable);
 
@@ -754,17 +749,23 @@ Interpreter::ResultAndReturnRegister Interpreter::run_executable(ExecutionContex
         }
     }
 
-    auto return_value = js_undefined();
-    if (!reg(Register::return_value()).is_special_empty_value())
-        return_value = reg(Register::return_value());
+    Value return_value;
+    if (auto return_register_value = reg(Register::return_value()); !return_register_value.is_special_empty_value())
+        return_value = return_register_value;
+    else {
+        return_value = reg(Register::accumulator());
+        if (return_value.is_special_empty_value())
+            return_value = js_undefined();
+    }
+
     auto exception = reg(Register::exception());
 
     vm().run_queued_promise_jobs();
     vm().finish_execution_generation();
 
-    if (!exception.is_special_empty_value())
-        return { throw_completion(exception), registers_and_constants_and_locals_and_arguments[0] };
-    return { return_value, registers_and_constants_and_locals_and_arguments[0] };
+    if (!exception.is_special_empty_value()) [[unlikely]]
+        return throw_completion(exception);
+    return return_value;
 }
 
 void Interpreter::enter_unwind_context()
diff --git a/Libraries/LibJS/Bytecode/Interpreter.h b/Libraries/LibJS/Bytecode/Interpreter.h
@@ -36,15 +36,10 @@ class JS_API Interpreter {
 
     ThrowCompletionOr<Value> run(ExecutionContext& context, Executable& executable, Optional<size_t> entry_point = {}, Value initial_accumulator_value = js_special_empty_value())
     {
-        auto result_and_return_register = run_executable(context, executable, entry_point, initial_accumulator_value);
-        return move(result_and_return_register.value);
+        return run_executable(context, executable, entry_point, initial_accumulator_value);
     }
 
-    struct ResultAndReturnRegister {
-        ThrowCompletionOr<Value> value;
-        Value return_register_value;
-    };
-    ResultAndReturnRegister run_executable(ExecutionContext&, Executable&, Optional<size_t> entry_point, Value initial_accumulator_value = js_special_empty_value());
+    ThrowCompletionOr<Value> run_executable(ExecutionContext&, Executable&, Optional<size_t> entry_point, Value initial_accumulator_value = js_special_empty_value());
 
     ALWAYS_INLINE Value& accumulator() { return reg(Register::accumulator()); }
     ALWAYS_INLINE Value& saved_return_value() { return reg(Register::saved_return_value()); }
diff --git a/Libraries/LibJS/Heap/Cell.h b/Libraries/LibJS/Heap/Cell.h
@@ -18,6 +18,8 @@ class JS_API Cell : public GC::Cell {
 public:
     virtual void initialize(Realm&);
 
+    virtual bool is_generator_result() const { return false; }
+
     ALWAYS_INLINE VM& vm() const { return *reinterpret_cast<VM*>(private_data()); }
 };
 
diff --git a/Libraries/LibJS/Runtime/AbstractOperations.cpp b/Libraries/LibJS/Runtime/AbstractOperations.cpp
@@ -735,11 +735,7 @@ ThrowCompletionOr<Value> perform_eval(VM& vm, Value x, CallerMode strict_caller,
 
     Optional<Value> eval_result;
 
-    auto result_or_error = vm.bytecode_interpreter().run_executable(*eval_context, *executable, {});
-    if (result_or_error.value.is_error())
-        return result_or_error.value.release_error();
-
-    eval_result = result_or_error.return_register_value;
+    eval_result = TRY(vm.bytecode_interpreter().run_executable(*eval_context, *executable, {}));
 
     // 32. If result.[[Type]] is normal and result.[[Value]] is empty, then
     //     a. Set result to NormalCompletion(undefined).
diff --git a/Libraries/LibJS/Runtime/AsyncGenerator.cpp b/Libraries/LibJS/Runtime/AsyncGenerator.cpp
@@ -156,13 +156,13 @@ void AsyncGenerator::execute(VM& vm, Completion completion)
     while (true) {
         // Loosely based on step 4 of https://tc39.es/ecma262/#sec-asyncgeneratorstart
         auto generated_value = [](Value value) -> Value {
-            if (value.is_cell())
+            if (value.is_cell() && value.as_cell().is_generator_result())
                 return static_cast<GeneratorResult const&>(value.as_cell()).result();
             return value.is_special_empty_value() ? js_undefined() : value;
         };
 
         auto generated_continuation = [&](Value value) -> Optional<size_t> {
-            if (value.is_cell()) {
+            if (value.is_cell() && value.as_cell().is_generator_result()) {
                 auto number_value = static_cast<GeneratorResult const&>(value.as_cell()).continuation();
                 if (number_value.is_null())
                     return {};
@@ -172,7 +172,7 @@ void AsyncGenerator::execute(VM& vm, Completion completion)
         };
 
         auto generated_is_await = [](Value value) -> bool {
-            if (value.is_cell())
+            if (value.is_cell() && value.as_cell().is_generator_result())
                 return static_cast<GeneratorResult const&>(value.as_cell()).is_await();
             return false;
         };
@@ -186,9 +186,8 @@ void AsyncGenerator::execute(VM& vm, Completion completion)
         // We should never enter `execute` again after the generator is complete.
         VERIFY(continuation_address.has_value());
 
-        auto next_result = bytecode_interpreter.run_executable(vm.running_execution_context(), *m_generating_function->bytecode_executable(), continuation_address, completion_cell);
+        auto result_value = bytecode_interpreter.run_executable(vm.running_execution_context(), *m_generating_function->bytecode_executable(), continuation_address, completion_cell);
 
-        auto result_value = move(next_result.value);
         if (!result_value.is_throw_completion()) {
             m_previous_value = result_value.release_value();
             auto value = generated_value(m_previous_value);
diff --git a/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp b/Libraries/LibJS/Runtime/ECMAScriptFunctionObject.cpp
@@ -823,7 +823,7 @@ void async_block_start(VM& vm, T const& async_body, PromiseCapability const& pro
             if (maybe_executable.is_error())
                 result = maybe_executable.release_error();
             else
-                result = vm.bytecode_interpreter().run_executable(vm.running_execution_context(), *maybe_executable.value(), {}).value;
+                result = vm.bytecode_interpreter().run_executable(vm.running_execution_context(), *maybe_executable.value(), {});
         }
         // c. Else,
         else {
@@ -886,13 +886,7 @@ template void async_function_start(VM&, PromiseCapability const&, GC::Function<C
 // 15.8.4 Runtime Semantics: EvaluateAsyncFunctionBody, https://tc39.es/ecma262/#sec-runtime-semantics-evaluatefunctionbody
 ThrowCompletionOr<Value> ECMAScriptFunctionObject::ordinary_call_evaluate_body(VM& vm, ExecutionContext& context)
 {
-    auto result_and_frame = vm.bytecode_interpreter().run_executable(context, *bytecode_executable(), {});
-
-    if (result_and_frame.value.is_error()) [[unlikely]] {
-        return result_and_frame.value.release_error();
-    }
-
-    auto result = result_and_frame.value.release_value();
+    auto result = TRY(vm.bytecode_interpreter().run_executable(context, *bytecode_executable(), {}));
 
     // NOTE: Running the bytecode should eventually return a completion.
     // Until it does, we assume "return" and include the undefined fallback from the call site.
diff --git a/Libraries/LibJS/Runtime/GeneratorObject.cpp b/Libraries/LibJS/Runtime/GeneratorObject.cpp
@@ -84,13 +84,13 @@ ThrowCompletionOr<GeneratorObject::IterationResult> GeneratorObject::execute(VM&
     // Loosely based on step 4 of https://tc39.es/ecma262/#sec-generatorstart mixed with https://tc39.es/ecma262/#sec-generatoryield at the end.
 
     auto generated_value = [](Value value) -> Value {
-        if (value.is_cell())
+        if (value.is_cell() && value.as_cell().is_generator_result())
             return static_cast<GeneratorResult const&>(value.as_cell()).result();
         return value.is_special_empty_value() ? js_undefined() : value;
     };
 
     auto generated_continuation = [&](Value value) -> Optional<size_t> {
-        if (value.is_cell()) {
+        if (value.is_cell() && value.as_cell().is_generator_result()) {
             auto number_value = static_cast<GeneratorResult const&>(value.as_cell()).continuation();
             if (number_value.is_null())
                 return {};
@@ -108,11 +108,10 @@ ThrowCompletionOr<GeneratorObject::IterationResult> GeneratorObject::execute(VM&
     // We should never enter `execute` again after the generator is complete.
     VERIFY(next_block.has_value());
 
-    auto next_result = bytecode_interpreter.run_executable(vm.running_execution_context(), *m_generating_function->bytecode_executable(), next_block, compleion_cell);
+    auto result_value = bytecode_interpreter.run_executable(vm.running_execution_context(), *m_generating_function->bytecode_executable(), next_block, compleion_cell);
 
     vm.pop_execution_context();
 
-    auto result_value = move(next_result.value);
     if (result_value.is_throw_completion()) {
         // Uncaught exceptions disable the generator.
         m_generator_state = GeneratorState::Completed;
diff --git a/Libraries/LibJS/Runtime/GeneratorResult.h b/Libraries/LibJS/Runtime/GeneratorResult.h
@@ -31,6 +31,7 @@ class GeneratorResult final : public Cell {
     [[nodiscard]] bool is_await() const { return m_is_await; }
 
 private:
+    virtual bool is_generator_result() const override { return true; }
     virtual void visit_edges(Cell::Visitor& visitor) override;
 
     bool m_is_await { false };
diff --git a/Libraries/LibJS/Runtime/ShadowRealm.cpp b/Libraries/LibJS/Runtime/ShadowRealm.cpp
@@ -175,12 +175,11 @@ ThrowCompletionOr<Value> perform_shadow_realm_eval(VM& vm, Value source, Realm&
     // 11. If result.[[Type]] is normal, then
     if (!eval_result.is_throw_completion()) {
         // a. Set result to the result of evaluating body.
-        auto result_and_return_register = vm.bytecode_interpreter().run_executable(*eval_context, *executable, {});
-        if (result_and_return_register.value.is_error()) {
-            result = result_and_return_register.value.release_error();
+        auto result_or_error = vm.bytecode_interpreter().run_executable(*eval_context, *executable, {});
+        if (result_or_error.is_error()) {
+            result = result_or_error.release_error();
         } else {
-            // Resulting value is in the accumulator.
-            result = result_and_return_register.return_register_value.is_special_empty_value() ? js_undefined() : result_and_return_register.return_register_value;
+            result = result_or_error.value();
         }
     }
 
diff --git a/Libraries/LibJS/SourceTextModule.cpp b/Libraries/LibJS/SourceTextModule.cpp
@@ -743,11 +743,11 @@ ThrowCompletionOr<void> SourceTextModule::execute_module(VM& vm, GC::Ptr<Promise
         // c. Let result be the result of evaluating module.[[ECMAScriptCode]].
         Completion result;
 
-        auto result_and_return_register = vm.bytecode_interpreter().run_executable(*module_context, *executable, {});
-        if (result_and_return_register.value.is_error()) {
-            result = result_and_return_register.value.release_error();
+        auto result_or_error = vm.bytecode_interpreter().run_executable(*module_context, *executable, {});
+        if (result_or_error.is_error()) {
+            result = result_or_error.release_error();
         } else {
-            result = result_and_return_register.return_register_value.is_special_empty_value() ? js_undefined() : result_and_return_register.return_register_value;
+            result = result_or_error.value().is_special_empty_value() ? js_undefined() : result_or_error.release_value();
         }
 
         // d. Let env be moduleContext's LexicalEnvironment.
