LibJS: Align NewPromiseCapability with spec changes

See https://github.com/tc39/ecma262/commit/874ecf9

After this refactoring, we now correctly handle non-function /
non-undefined objects being passed multiple times: instead of skipping
assignment to promiseCapability altogether and failing with a
NotAFunction error in the end; on the second time the executor closure
is called, we return GetCapabilitiesExecutorCalledMultipleTimes.

This fixes the 7 `capability-executor-called-twice.js` test262 tests.
This commit is contained in:
Daniel Bertalan 2023-06-28 23:38:45 +02:00 committed by Linus Groh
parent 0748e755d8
commit c21255da7f
Notes: sideshowbarker 2024-07-17 01:51:00 +09:00
2 changed files with 40 additions and 38 deletions

View file

@ -11,12 +11,12 @@
namespace JS { namespace JS {
NonnullGCPtr<PromiseCapability> PromiseCapability::create(VM& vm, GCPtr<Object> promise, GCPtr<FunctionObject> resolve, GCPtr<FunctionObject> reject) NonnullGCPtr<PromiseCapability> PromiseCapability::create(VM& vm, NonnullGCPtr<Object> promise, NonnullGCPtr<FunctionObject> resolve, NonnullGCPtr<FunctionObject> reject)
{ {
return vm.heap().allocate_without_realm<PromiseCapability>(promise, resolve, reject); return vm.heap().allocate_without_realm<PromiseCapability>(promise, resolve, reject);
} }
PromiseCapability::PromiseCapability(GCPtr<Object> promise, GCPtr<FunctionObject> resolve, GCPtr<FunctionObject> reject) PromiseCapability::PromiseCapability(NonnullGCPtr<Object> promise, NonnullGCPtr<FunctionObject> resolve, NonnullGCPtr<FunctionObject> reject)
: m_promise(promise) : m_promise(promise)
, m_resolve(resolve) , m_resolve(resolve)
, m_reject(reject) , m_reject(reject)
@ -31,6 +31,22 @@ void PromiseCapability::visit_edges(Cell::Visitor& visitor)
visitor.visit(m_reject); visitor.visit(m_reject);
} }
namespace {
struct ResolvingFunctions final : public Cell {
JS_CELL(ResolvingFunctions, Cell);
Value resolve { js_undefined() };
Value reject { js_undefined() };
virtual void visit_edges(Cell::Visitor& visitor) override
{
Base::visit_edges(visitor);
visitor.visit(resolve);
visitor.visit(reject);
}
};
}
// 27.2.1.5 NewPromiseCapability ( C ), https://tc39.es/ecma262/#sec-newpromisecapability // 27.2.1.5 NewPromiseCapability ( C ), https://tc39.es/ecma262/#sec-newpromisecapability
ThrowCompletionOr<NonnullGCPtr<PromiseCapability>> new_promise_capability(VM& vm, Value constructor) ThrowCompletionOr<NonnullGCPtr<PromiseCapability>> new_promise_capability(VM& vm, Value constructor)
{ {
@ -42,34 +58,28 @@ ThrowCompletionOr<NonnullGCPtr<PromiseCapability>> new_promise_capability(VM& vm
// 2. NOTE: C is assumed to be a constructor function that supports the parameter conventions of the Promise constructor (see 27.2.3.1). // 2. NOTE: C is assumed to be a constructor function that supports the parameter conventions of the Promise constructor (see 27.2.3.1).
// 3. Let promiseCapability be the PromiseCapability Record { [[Promise]]: undefined, [[Resolve]]: undefined, [[Reject]]: undefined }. // 3. Let resolvingFunctions be the Record { [[Resolve]]: undefined, [[Reject]]: undefined }.
auto promise_capability = PromiseCapability::create(vm, nullptr, nullptr, nullptr); auto resolving_functions = TRY(vm.heap().allocate<ResolvingFunctions>(realm));
// 4. Let executorClosure be a new Abstract Closure with parameters (resolve, reject) that captures promiseCapability and performs the following steps when called: // 4. Let executorClosure be a new Abstract Closure with parameters (resolve, reject) that captures resolvingFunctions and performs the following steps when called:
// NOTE: Additionally to capturing the GC-allocated promise capability, we also create handles for auto executor_closure = [resolving_functions](auto& vm) -> ThrowCompletionOr<Value> {
// the resolve and reject values to keep them alive within the closure, as it may outlive the former.
auto executor_closure = [&promise_capability, resolve_handle = make_handle({}), reject_handle = make_handle({})](auto& vm) mutable -> ThrowCompletionOr<Value> {
auto resolve = vm.argument(0); auto resolve = vm.argument(0);
auto reject = vm.argument(1); auto reject = vm.argument(1);
// No idea what other engines say here. // No idea what other engines say here.
// a. If promiseCapability.[[Resolve]] is not undefined, throw a TypeError exception. // a. If promiseCapability.[[Resolve]] is not undefined, throw a TypeError exception.
if (!resolve_handle.is_null()) if (!resolving_functions->resolve.is_undefined())
return vm.template throw_completion<TypeError>(ErrorType::GetCapabilitiesExecutorCalledMultipleTimes); return vm.template throw_completion<TypeError>(ErrorType::GetCapabilitiesExecutorCalledMultipleTimes);
// b. If promiseCapability.[[Reject]] is not undefined, throw a TypeError exception. // b. If promiseCapability.[[Reject]] is not undefined, throw a TypeError exception.
if (!reject_handle.is_null()) if (!resolving_functions->reject.is_undefined())
return vm.template throw_completion<TypeError>(ErrorType::GetCapabilitiesExecutorCalledMultipleTimes); return vm.template throw_completion<TypeError>(ErrorType::GetCapabilitiesExecutorCalledMultipleTimes);
// c. Set promiseCapability.[[Resolve]] to resolve. // c. Set promiseCapability.[[Resolve]] to resolve.
resolve_handle = make_handle(resolve); resolving_functions->resolve = resolve;
if (resolve.is_function())
promise_capability->set_resolve(resolve.as_function());
// d. Set promiseCapability.[[Reject]] to reject. // d. Set promiseCapability.[[Reject]] to reject.
reject_handle = make_handle(reject); resolving_functions->reject = reject;
if (reject.is_function())
promise_capability->set_reject(reject.as_function());
// e. Return undefined. // e. Return undefined.
return js_undefined(); return js_undefined();
@ -81,21 +91,16 @@ ThrowCompletionOr<NonnullGCPtr<PromiseCapability>> new_promise_capability(VM& vm
// 6. Let promise be ? Construct(C, « executor »). // 6. Let promise be ? Construct(C, « executor »).
auto promise = TRY(construct(vm, constructor.as_function(), executor)); auto promise = TRY(construct(vm, constructor.as_function(), executor));
// 7. If IsCallable(promiseCapability.[[Resolve]]) is false, throw a TypeError exception. // 7. If IsCallable(resolvingFunctions.[[Resolve]]) is false, throw a TypeError exception.
// NOTE: We only assign a value in the executor closure if it is a function. if (!resolving_functions->resolve.is_function())
if (promise_capability->resolve() == nullptr)
return vm.throw_completion<TypeError>(ErrorType::NotAFunction, "Promise capability resolve value"); return vm.throw_completion<TypeError>(ErrorType::NotAFunction, "Promise capability resolve value");
// 8. If IsCallable(promiseCapability.[[Reject]]) is false, throw a TypeError exception. // 8. If IsCallable(resolvingFunctions.[[Reject]]) is false, throw a TypeError exception.
// NOTE: We only assign a value in the executor closure if it is a function. if (!resolving_functions->resolve.is_function())
if (promise_capability->reject() == nullptr)
return vm.throw_completion<TypeError>(ErrorType::NotAFunction, "Promise capability reject value"); return vm.throw_completion<TypeError>(ErrorType::NotAFunction, "Promise capability reject value");
// 9. Set promiseCapability.[[Promise]] to promise. // 9. Return the PromiseCapability Record { [[Promise]]: promise, [[Resolve]]: resolvingFunctions.[[Resolve]], [[Reject]]: resolvingFunctions.[[Reject]] }.
promise_capability->set_promise(*promise); return PromiseCapability::create(vm, promise, resolving_functions->resolve.as_function(), resolving_functions->reject.as_function());
// 10. Return promiseCapability.
return promise_capability;
} }
} }

View file

@ -17,27 +17,24 @@ class PromiseCapability final : public Cell {
JS_CELL(PromiseCapability, Cell); JS_CELL(PromiseCapability, Cell);
public: public:
static NonnullGCPtr<PromiseCapability> create(VM& vm, GCPtr<Object> promise, GCPtr<FunctionObject> resolve, GCPtr<FunctionObject> reject); static NonnullGCPtr<PromiseCapability> create(VM& vm, NonnullGCPtr<Object> promise, NonnullGCPtr<FunctionObject> resolve, NonnullGCPtr<FunctionObject> reject);
virtual ~PromiseCapability() = default; virtual ~PromiseCapability() = default;
[[nodiscard]] GCPtr<Object> promise() const { return m_promise; } [[nodiscard]] NonnullGCPtr<Object> promise() const { return m_promise; }
void set_promise(NonnullGCPtr<Object> promise) { m_promise = promise; }
[[nodiscard]] GCPtr<FunctionObject> resolve() const { return m_resolve; } [[nodiscard]] NonnullGCPtr<FunctionObject> resolve() const { return m_resolve; }
void set_resolve(NonnullGCPtr<FunctionObject> resolve) { m_resolve = resolve; }
[[nodiscard]] GCPtr<FunctionObject> reject() const { return m_reject; } [[nodiscard]] NonnullGCPtr<FunctionObject> reject() const { return m_reject; }
void set_reject(NonnullGCPtr<FunctionObject> reject) { m_reject = reject; }
private: private:
PromiseCapability(GCPtr<Object>, GCPtr<FunctionObject>, GCPtr<FunctionObject>); PromiseCapability(NonnullGCPtr<Object>, NonnullGCPtr<FunctionObject>, NonnullGCPtr<FunctionObject>);
virtual void visit_edges(Visitor&) override; virtual void visit_edges(Visitor&) override;
GCPtr<Object> m_promise; NonnullGCPtr<Object> m_promise;
GCPtr<FunctionObject> m_resolve; NonnullGCPtr<FunctionObject> m_resolve;
GCPtr<FunctionObject> m_reject; NonnullGCPtr<FunctionObject> m_reject;
}; };
// 27.2.1.1.1 IfAbruptRejectPromise ( value, capability ), https://tc39.es/ecma262/#sec-ifabruptrejectpromise // 27.2.1.1.1 IfAbruptRejectPromise ( value, capability ), https://tc39.es/ecma262/#sec-ifabruptrejectpromise