LibJS: Remove unneeded FIXMEs for suspending an execution context

From what I understand, the suspension steps are not required now,
or in the future for our implementation, or any other. The intent
is already implemented in the spec pushing on another execution
context to the stack and leaving the running execution context as-is.

The resume steps are a slightly different story as there is some subtle
behavior which the spec is trying to convey where some custom logic may
need to be done when one execution context changes from one to another.
It may be worth implementing those steps at a later point in time so
that this behavior is a bit easier to follow in those cases.

To make the situation more confusing - from what I can gather from the
spec, not all cases that the spec mentions resume actually means
anything normative. Resume is only _actually_ needed in a limited set
of locations.

For now, let's just remove the unneeded FIXMEs that indicate that there
is something to be done for the suspension steps, as there is not, and
leave the resume steps as is.
This commit is contained in:
Shannon Booth 2025-01-01 22:51:52 +13:00 committed by Andreas Kling
parent 44bb2b7e32
commit d48a0aaa55
Notes: github-actions[bot] 2025-01-02 10:31:16 +00:00
9 changed files with 15 additions and 30 deletions

View file

@ -216,8 +216,7 @@ ThrowCompletionOr<Value> Interpreter::run(Script& script_record, GC::Ptr<Environ
// NOTE: This isn't in the spec, but we require it.
script_context->is_strict_mode = script_record.parse_node().is_strict_mode();
// FIXME: 9. Suspend the currently running execution context.
// 9. Suspend the currently running execution context.
// 10. Push scriptContext onto the execution context stack; scriptContext is now the running execution context.
TRY(vm.push_execution_context(*script_context, {}));
@ -260,7 +259,7 @@ ThrowCompletionOr<Value> Interpreter::run(Script& script_record, GC::Ptr<Environ
result = normal_completion(js_undefined());
}
// FIXME: 15. Suspend scriptContext and remove it from the execution context stack.
// 15. Suspend scriptContext and remove it from the execution context stack.
vm.pop_execution_context();
// 16. Assert: The execution context stack is not empty.

View file

@ -639,7 +639,7 @@ ThrowCompletionOr<Value> perform_eval(VM& vm, Value x, CallerMode strict_caller,
}
// 19. If runningContext is not already suspended, suspend runningContext.
// FIXME: We don't have this concept yet.
// NOTE: Done by the push on step 27.
// 20. Let evalContext be a new ECMAScript code execution context.
auto eval_context = ExecutionContext::create();
@ -670,8 +670,7 @@ ThrowCompletionOr<Value> perform_eval(VM& vm, Value x, CallerMode strict_caller,
// NOTE: We use a ScopeGuard to automatically pop the execution context when any of the `TRY`s below return a throw completion.
ScopeGuard pop_guard = [&] {
// FIXME: 31. Suspend evalContext and remove it from the execution context stack.
// 31. Suspend evalContext and remove it from the execution context stack.
// 32. Resume the context that is now on the top of the execution context stack as the running execution context.
vm.pop_execution_context();
};

View file

@ -57,8 +57,7 @@ ThrowCompletionOr<void> AsyncFunctionDriverWrapper::await(JS::Value value)
// a. Let prevContext be the running execution context.
auto& prev_context = vm.running_execution_context();
// FIXME: b. Suspend prevContext.
// b. Suspend prevContext.
// c. Push asyncContext onto the execution context stack; asyncContext is now the running execution context.
TRY(vm.push_execution_context(*m_suspended_execution_context, {}));
@ -85,8 +84,7 @@ ThrowCompletionOr<void> AsyncFunctionDriverWrapper::await(JS::Value value)
// a. Let prevContext be the running execution context.
auto& prev_context = vm.running_execution_context();
// FIXME: b. Suspend prevContext.
// b. Suspend prevContext.
// c. Push asyncContext onto the execution context stack; asyncContext is now the running execution context.
TRY(vm.push_execution_context(*m_suspended_execution_context, {}));

View file

@ -87,8 +87,7 @@ ThrowCompletionOr<void> AsyncGenerator::await(Value value)
// a. Let prevContext be the running execution context.
auto& prev_context = vm.running_execution_context();
// FIXME: b. Suspend prevContext.
// b. Suspend prevContext.
// c. Push asyncContext onto the execution context stack; asyncContext is now the running execution context.
TRY(vm.push_execution_context(async_context, {}));
@ -115,8 +114,7 @@ ThrowCompletionOr<void> AsyncGenerator::await(Value value)
// a. Let prevContext be the running execution context.
auto& prev_context = vm.running_execution_context();
// FIXME: b. Suspend prevContext.
// b. Suspend prevContext.
// c. Push asyncContext onto the execution context stack; asyncContext is now the running execution context.
TRY(vm.push_execution_context(async_context, {}));
@ -309,8 +307,7 @@ ThrowCompletionOr<void> AsyncGenerator::resume(VM& vm, Completion completion)
// 3. Let callerContext be the running execution context.
auto const& caller_context = vm.running_execution_context();
// FIXME: 4. Suspend callerContext.
// 4. Suspend callerContext.
// 5. Set generator.[[AsyncGeneratorState]] to executing.
m_async_generator_state = State::Executing;

View file

@ -623,8 +623,6 @@ ThrowCompletionOr<void> ECMAScriptFunctionObject::prepare_for_ordinary_call(Exec
callee_context.private_environment = m_private_environment;
// 11. If callerContext is not already suspended, suspend callerContext.
// FIXME: We don't have this concept yet.
// 12. Push calleeContext onto the execution context stack; calleeContext is now the running execution context.
TRY(vm.push_execution_context(callee_context, {}));

View file

@ -146,8 +146,7 @@ ThrowCompletionOr<Value> GeneratorObject::resume(VM& vm, Value value, Optional<S
// 5. Let methodContext be the running execution context.
auto const& method_context = vm.running_execution_context();
// FIXME: 6. Suspend methodContext.
// 6. Suspend methodContext.
// 8. Push genContext onto the execution context stack; genContext is now the running execution context.
// NOTE: This is done out of order as to not permanently disable the generator if push_execution_context throws,
// as `resume` will immediately throw when [[GeneratorState]] is "executing", never allowing the state to change.
@ -208,8 +207,7 @@ ThrowCompletionOr<Value> GeneratorObject::resume_abrupt(JS::VM& vm, JS::Completi
// 6. Let methodContext be the running execution context.
auto const& method_context = vm.running_execution_context();
// FIXME: 7. Suspend methodContext.
// 7. Suspend methodContext.
// 9. Push genContext onto the execution context stack; genContext is now the running execution context.
// NOTE: This is done out of order as to not permanently disable the generator if push_execution_context throws,
// as `resume_abrupt` will immediately throw when [[GeneratorState]] is "executing", never allowing the state to change.

View file

@ -118,8 +118,6 @@ ThrowCompletionOr<Value> NativeFunction::internal_call(Value this_argument, Read
auto& caller_context = vm.running_execution_context();
// 2. If callerContext is not already suspended, suspend callerContext.
// NOTE: We don't support this concept yet.
// 3. Let calleeContext be a new execution context.
auto callee_context = ExecutionContext::create();
@ -181,8 +179,6 @@ ThrowCompletionOr<GC::Ref<Object>> NativeFunction::internal_construct(ReadonlySp
auto& caller_context = vm.running_execution_context();
// 2. If callerContext is not already suspended, suspend callerContext.
// NOTE: We don't support this concept yet.
// 3. Let calleeContext be a new execution context.
auto callee_context = ExecutionContext::create();

View file

@ -702,7 +702,7 @@ ThrowCompletionOr<void> SourceTextModule::execute_module(VM& vm, GC::Ptr<Promise
module_context->lexical_environment = environment();
// 8. Suspend the currently running execution context.
// FIXME: We don't have suspend yet
// NOTE: Done by the push of execution context in steps below.
// 9. If module.[[HasTLA]] is false, then
if (!m_has_top_level_await) {
@ -760,8 +760,8 @@ ThrowCompletionOr<void> SourceTextModule::execute_module(VM& vm, GC::Ptr<Promise
// the top-level module code.
// FIXME: Improve this situation, so we can match the spec better.
// AD-HOC: We push/pop the moduleContext around the function construction to ensure that the async execution context
// captures the module execution context.
// NOTE: Like AsyncBlockStart, we need to push/pop the moduleContext around the function construction to ensure that
// the async execution context captures the module execution context.
vm.push_execution_context(*module_context);
FunctionParsingInsights parsing_insights;

View file

@ -78,7 +78,7 @@ ThrowCompletionOr<Promise*> SyntheticModule::evaluate(VM& vm)
{
// Note: Has some changes from PR: https://github.com/tc39/proposal-json-modules/pull/13.
// 1. Suspend the currently running execution context.
// FIXME: We don't have suspend yet.
// NOTE: Done by the push on step 8.
// 2. Let moduleContext be a new ECMAScript code execution context.
auto module_context = ExecutionContext::create();